Page MenuHomeElementl

Update Scheduler helm schema to use enum types
ClosedPublic

Authored by rexledesma on Oct 27 2020, 5:52 PM.

Details

Summary

Depends on D5748

As the title, enumify the helm scheduler values, rather
than relying on a boolean toggle.

Test Plan

bk/helm lint
integration

Diff Detail

Repository
R1 dagster
Branch
rl-json-schema-scheduler
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

rexledesma retitled this revision from [3/n] Add schema for scheduler to [2/n] Add helm schema for Scheduler configuration.Oct 28 2020, 12:20 AM
rexledesma edited the summary of this revision. (Show Details)
rexledesma retitled this revision from [2/n] Add helm schema for Scheduler configuration to [2/n] Add helm schema validation for Scheduler configuration.Oct 28 2020, 1:28 AM
rexledesma added a reviewer: Restricted Project.Oct 28 2020, 6:52 PM
rexledesma added inline comments.
python_modules/automation/automation/helm/schema/scheduler.py
39

support for conditional schemas isn't first class in pydantic so we'll have to override the schema here (https://github.com/samuelcolvin/pydantic/issues/529)

alangenfeld added inline comments.
helm/dagster/templates/configmap-instance.yaml
13–14

this is a breaking change right? do we want to do that ahead of 0.10.0?

python_modules/automation/automation/helm/schema/scheduler.py
39

maybe leave this is a comment in the code

separate out breaking changes from things we can schema-tize without breaking

This revision now requires changes to proceed.Oct 29 2020, 6:29 PM

Will wait until cherry pick mode to land this - removing reviewers for now

This revision now requires review to proceed.Nov 2 2020, 7:35 PM

necro

resurrect from the dead

rexledesma removed a subscriber: alangenfeld.

make sure to document all the breaking changes in changes.md and migration.md

rexledesma retitled this revision from [2/n] Add helm schema validation for Scheduler configuration to Update Scheduler helm schema to use enum types.Jan 6 2021, 5:01 AM
rexledesma edited the summary of this revision. (Show Details)
rexledesma added a child revision: Restricted Differential Revision.Jan 8 2021, 4:54 PM
alangenfeld added inline comments.
helm/dagster/templates/configmap-instance.yaml
44–45

probably a custom option too just to be safe that takes module class and config?

This revision is now accepted and ready to land.Jan 8 2021, 7:31 PM