Page MenuHomeElementl

fix: assert compute log manager json schema matches configurable class
ClosedPublic

Authored by rexledesma on Jun 18 2021, 3:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 26, 8:23 AM
Unknown Object (File)
Mon, Mar 20, 7:38 AM
Unknown Object (File)
Fri, Mar 17, 1:20 PM
Unknown Object (File)
Fri, Mar 10, 10:04 PM
Unknown Object (File)
Feb 9 2023, 6:15 AM
Unknown Object (File)
Feb 8 2023, 2:39 PM
Unknown Object (File)
Feb 7 2023, 7:58 AM
Unknown Object (File)
Feb 6 2023, 11:44 PM
Subscribers
None

Diff Detail

Repository
R1 dagster
Branch
rl/test-compute-logs
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

helm/dagster/schema/schema_tests/test_instance.py
101

So if we were to ever add another compute log manager, we'd have to remember to go add it here?

Are there other configurable classes we should do this for as well?

helm/dagster/schema/tox.ini
8

I think I understand why aws, azure, and gcp are getting added (although I don't love what it says about our dependency tree that our general dagster helm chart depends on our specific cloud provider packages). Why are pandas and postgres getting added? Because they're dependencies of one of these other packages?

helm/dagster/schema/schema_tests/test_instance.py
101

Yes.. do you think that's a reasonable request? I'm trying to get the best of both worlds where the config is typed but also ensure that there's no drift between the OSS configuration and the Helm configuration. If this is too much of a headache, we can go back to the original plan of just removing the config typing

helm/dagster/schema/tox.ini
8

Otherwise, the command below pip list --exclude-editable | grep -e dagster -e dagit' would fail since the pypi versions of these packages would be installed. Yeah, definitely unideal

dgibson added inline comments.
helm/dagster/schema/schema_tests/test_instance.py
101

no strong opinion here, I guess the main thing this helps with is preventing typos in the class name? ultimately i think my gut would be to make it permissive for things that users are allowed to change, but I don't feel strongly

This revision is now accepted and ready to land.Jun 21 2021, 5:31 PM
This revision was landed with ongoing or failed builds.Jun 21 2021, 9:48 PM
This revision was automatically updated to reflect the committed changes.