Page MenuHomeElementl

Improve error messages for celery-k8s launcher
ClosedPublic

Authored by sashank on Dec 7 2020, 5:56 PM.

Details

Summary

This diff improves the error messages when using the celery-k8s executor + launcher, specifically for the error cases when:

  • The user specifies a job_image when using user-code deployments
  • The user tries to use the configured API for the executor
Test Plan

integration
unit

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sashank created this revision.

up

python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py
177–182

When does this actually happen? Is this even a possible error case, or is this an internal check?

191–197

When does this actually happen? Is this even a possible error case, or is this an internal check?

python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py
177–182

Ignore this message, moved comment to correct spot below.

sashank published this revision for review.Dec 7 2020, 6:04 PM
python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py
212

i think we're currently being too strict and we actually want to support passing in a job_image even if the user has user code deployments. See https://github.com/dagster-io/dagster/issues/3286

Integration failures

alangenfeld added a subscriber: catherinewu.

I agree with @catherinewu that we should support a provided job image and maybe emit an engine event or log that we are overwriting the user code deploy image or something

that said this diff as is a strict improvement over the current error experience

This revision is now accepted and ready to land.Dec 14 2020, 5:18 PM
python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py
212

I missed this comment, good call. Will add that in a follow up.