- tweaked test because c/i has no api key installed
- renamed dagster_components to common
- got rid of errant test file
jesus christ good catch.....pytest refactor is not as smart as I thought.
ready for a once over! Good call on the naming change and the container switch, this actually makes my general life a lot easier as well since I switched my dagster-home to point to the test-postgres-db.
"risky" might be the wrong word but more like "too easy to break" meaning that anytime you change that config key it will certainly not behave the way you want to. Any change with the config would have to be coupled with a change to code. Given that it doesn't seem like something that should be in config
Exposing a configurable priority key is actually pretty risky and will result in a lot of replicated config.
- renamed dagster_components to common and switched to using the test-postgres-db container
So the crux of it is:
- we do dagster/priority and it works for the core executors but (probably?) none of the others which leads to an unfortunate expectation mis-match in a not clearly communicated way (keys just ignored)
- we do separate keys for each executor to align expectations ie dagster/in-process/priority, dagster-in-process/priority, whatever but thats aesthetically unfortunate and leads to having to set all keys to support different execution modes
what should the predefined keys be for the in process and multi process engines? I wasn't happy with what I came up with which motivated me to punt it out to config
So what I was expecting to see here is not a configurable priority key in terms of *configuring* executor on a per-run basis, but instead just having it be part of the executor definition, plus a comparator functions between priorities. Exposing a configurable priority key is actually pretty risky and will result in a lot of replicated config.
move sorting out to individual engines. use config to drive what key to use for prioirty on default engines
as in, not having the migration seems fine, when you can accomplish the same thing by running wipe and up
I think it should be fine, but we should add something to the changelog, saying "Need to run dagster schedule ..."