Add target Datasets to gardener configuration

m-lab / etl-gardener

Gardener provides services for maintaining and reprocessing mlab data.

Apache License 2.0

13 stars 5 forks source link

Add target Datasets to gardener configuration #390

Closed stephen-soltesz closed 1 year ago

stephen-soltesz commented 1 year ago

This change adds target datasets to the gardener configuration, making the target output datasets locations controlled entirely by configuration rather than static constants in the gardener code and query templates.

With this change, it will be possible to configure both the gs://archive-measurement-lab source as well as project local archive buckets in sandbox and staging environments (if desired), or to add multiple output locations for development. It will also be possible to output "versioned tables" for breaking schema changes, either with the version in the dataset, e.g. v2_ndt.ndt7 or with a future change that adds the version as a suffix to the base table, e.g. ndt.ndt7_v2.

This change makes the set of tables "joined" with annotation data implicit with the Datasets.Join field. Datatypes without this field will not be joined.

Part of:

https://github.com/m-lab/etl-gardener/issues/349

This change is

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3618

23 of 28 (82.14%) changed or added relevant lines in 6 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.8%) to 78.442%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cloud/bq/ops.go	1	6	16.67%
<!--	Total:	23	28	82.14%	-->

Totals
Change from base Build 3603:	0.8%
Covered Lines:	1259
Relevant Lines:	1605

💛 - Coveralls

stephen-soltesz commented 1 year ago

@SaiedKazemi this PR is still a draft, but I would welcome your early feedback.

SaiedKazemi commented 1 year ago

I am not familiar with this repo and do not have much context but the changes looked good to me. Can you please specify how you tested the changes in the commit message?

stephen-soltesz commented 1 year ago

@cristinaleonr I would welcome your feedback as well.

stephen-soltesz commented 1 year ago

Also FYI: @mattmathis - this is one way of supporting "versioned tables" through gardener configuration.

stephen-soltesz commented 1 year ago

Okay, I've left this PR as draft b/c there are two down stream dependencies to use this feature:

the Saver of an earlier gardener version will save Jobs without the Datasets, a datasets-enabled version would load incomplete information. So we need a migration path.
etl parser writes to an output bucket, but does not include the archive bucket source in the GCS path, only the prefix. By default, this would mix two archive sources into the same GCS output prefix, e.g. ndt/ndt7/2022/01/01/* would include archives from archive-mlab-sandbox and archive-measurement-lab if both were configured sources. I think we want to include the archive source in the output path, e.g. gs://etl-mlab-sandbox/archive-mlab-sandbox/ndt/ndt7/2022/**

mattmathis commented 1 year ago

Are you using data set table names like tmp_ndt or ndt_tmp? The documentation and views currency use the latter, but point to the former. Is this OK?

stephen-soltesz commented 1 year ago

@cristinaleonr after a long interlude of dependencies now completed, this change is much more modest than how it started. When you have time, PTAL?