Page MenuHomePhabricator

Helm: annotations everywhere
ClosedPublic

Authored by nate on Dec 29 2020, 4:28 AM.

Details

Summary

Fixes #3050

Test Plan

integration

Added a test which annotates resources, then uses the kubernetes APIs to confirm that the resources are tagged correctly

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 29 2020, 4:31 AM
Harbormaster failed remote builds in B23511: Diff 28587!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 29 2020, 5:29 AM
Harbormaster failed remote builds in B23512: Diff 28588!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 29 2020, 7:21 AM
Harbormaster failed remote builds in B23515: Diff 28591!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 29 2020, 5:27 PM
Harbormaster failed remote builds in B23516: Diff 28592!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 29 2020, 6:29 PM
Harbormaster failed remote builds in B23521: Diff 28597!
python_modules/automation/automation/helm/schema/subschema/dagit.py
5

just making this import style consistent with other files in this folder

nate edited the test plan for this revision. (Show Details)
nate added reviewers: rexledesma, catherinewu, johann.
nate requested review of this revision.Dec 29 2020, 7:00 PM

Awesome! Small nit: we validate that userDeployments.service.annotations exists but assume that dagit.service.annotations and flower.service.annotations exist -- would prefer either assuming all or validating all?

This revision is now accepted and ready to land.Jan 4 2021, 6:00 PM

Small nit: we validate that userDeployments.service.annotations exists but assume that dagit.service.annotations and flower.service.annotations exist -- would prefer either assuming all or validating all?

yeah, I added the validation this way because $deployment.service comes from the user and may not be specified, while our default values.yaml specifies dagit.service.annotations and flower.service.annotations - so if the user doesn't specify they're still going to be there as empty dicts

This revision was automatically updated to reflect the committed changes.