refactor: cache with context

[iwpnd]

to merge after #1012

[gdey]

LGTM, just a small nit about import lines.

I need to look at the CI issues as well. Looks like it's an issue with valun, and maybe how vendoring is done?

[iwpnd]

LGTM, just a small nit about import lines.

I need to look at the CI issues as well. Looks like it's an issue with valun, and maybe how vendoring is done?

that'll be fine after #1012

[ARolek]

Thank you for tackling this! Tegola is much better off with this change. I have a couple minor requests inline, but they're not show stoppers.

[ARolek]

Consider passing context to the New() call. The CLI framework we're using, cobra, has a context on cmd.Context()

[iwpnd]

Hm but caches are initialized in their respective init not in the root cmd. I don’t see how adding the cmd context makes sense here. 🤔

[ARolek]

I just walked through the code and the cache init routine is a bit confusing. The part I'm referencing enhancing with the context is the InitFunc:

tegola/cache/cache.go

Line 136 in 93b917e

type InitFunc func(dict.Dicter) (Interface, error)

This is called by the For method:

tegola/cache/cache.go

Line 165 in 93b917e

func For(cacheType string, config dict.Dicter) (Interface, error) {

Which is invoked by the internal cmd/internal/register/ package:

tegola/cmd/internal/register/caches.go

Line 16 in 93b917e

func Cache(config dict.Dicter) (cache.Interface, error) {

The main value we would get with this plumbing is if tegola is started up and then killed before the cache initializes fully.

I'm not going to block the PR on this request, but just following up on the intent of my comment. Your call if you want to make this change ;-)

[iwpnd]

gotcha! if thats fine with you i'd merge this as is, and tackle this in a separate commit.

[ARolek]

@iwpnd can you rebase against master and fix the conflicts please?

[coveralls]

Pull Request Test Coverage Report for Build b6a8fe572-PR-1013

Details

• 18 of 68 (26.47%) changed or added relevant lines in 10 files are covered.
• 8 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.06%) to 42.844%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cmd/tegola/cmd/cache/worker.go	0	3	0.0%
atlas/atlas.go	0	6	0.0%
cache/file/file.go	7	13	53.85%
cache/s3/s3.go	3	9	33.33%
observability/prometheus/cache.go	0	6	0.0%
cache/gcs/gcs.go	0	11	0.0%
cache/azblob/azblob.go	0	12	0.0%

Files with Coverage Reduction	New Missed Lines	%
atlas/atlas.go	1	3.62%
cache/redis/redis.go	7	92.31%

Totals
Change from base Build 93b917e04:	0.06%
Covered Lines:	7026
Relevant Lines:	16399

---

💛 - Coveralls