Rename and remove args that can be gathered from the pipeline_run
Details
Integration
foo <- newline necessary to trigger tests?
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
if someone upgrade their user code but forgets to update the celery workers - how does this fail? We don't promise cross version compat so i think this "breaking" change is probably ok - as long as it fails in a clear way that someone knows what to do about it
Could I get another quick look at this before landing since it's been sitting around
if someone upgrade their user code but forgets to update the celery workers - how does this fail? We don't promise cross version compat so i think this "breaking" change is probably ok - as long as it fails in a clear way that someone knows what to do about it
Update user code -> run worker (former run coordinator) spins up with the new code, and makes a celery call with the new args -> task apply fails, raises error about signature
and i think if we just kept this around that points directly at the new execute_step one, we may be totally fine
I don't think this helps in the k8s case due to the run worker using the user code as well (user code -> celery -> user code, so we'll break on celery signature on the first hop). I think the case where it does help is with dagster-celery-docker, where the docker image could be newer than the celery worker calling it. If it's older than the worker calling it, we'd break since execute_step won't be available.
given the "breaking change" adjacent potential - should we hold off on landing this til branch cut since were close (potentially after the next release)?
python_modules/dagster/dagster/cli/api.py | ||
---|---|---|
218–219 | the execute_run_with... does have external callers - custom run launchers, so keep that in mind for that one |