-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support more options for new experiments #1113
Conversation
Pull Request Test Coverage Report for Build 7460
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this PR is not starting it, but my opinion is that this command is doing way too many things.
Ideally, only the main()
function would be here, and the rest would be in a separate package with unit tests.
Reviewable status: 0 of 1 approvals obtained (waiting on @stephen-soltesz)
cmd/update-schema/update.go
line 190 at r2 (raw file):
if !ok { log.Printf("failed to find %v", table) return 1
Should this increase the error count and continue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely agree. How about this:
- IOU - migration of this functionality to etl-gardener repo, plus refactoring logic into main + supporting packages with unit tests.
Reviewable status: 0 of 1 approvals obtained (waiting on @cristinaleonr)
cmd/update-schema/update.go
line 190 at r2 (raw file):
Previously, cristinaleonr (Cristina Leon) wrote…
Should this increase the error count and continue?
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 1 approvals obtained
Thank you! |
Previously, the
update-schema
utility only supported legacy datatypes andndt
sidecars. The command assumed that datasets already existed. And the logic was straight forward but very redundant.This change adds support to
update-schema
for creating datasets and sidecar tables for alternate experiments. Previous behavior is still available but no longer run by default. Now, the-standard
and-legacy
options are independent. We typically do not need to update the legacy tables any longer and the template table iteration is time consuming.Example usage now includes:
This change is