Page MenuHomeElementl
Differential D5456

Improve error messages for celery-k8s launcher
ClosedPublic
Actions

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

Details

Reviewers
alangenfeld
• nate
catherinewu
Commits
R1:84d5305ac23c: Improve error messages for celery-k8s launcher
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 updated this revision to Diff 27129.Dec 7 2020, 5:56 PM
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?

sashank added inline comments.Dec 7 2020, 5:58 PM
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
catherinewu added inline comments.Dec 7 2020, 6:07 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

Harbormaster failed remote builds in B22326: Diff 27128!Dec 7 2020, 6:13 PM
sashank planned changes to this revision.Dec 7 2020, 6:14 PM

Integration failures

Harbormaster failed remote builds in B22327: Diff 27129!Dec 7 2020, 6:16 PM
sashank updated this revision to Diff 27137.Dec 7 2020, 6:23 PM

up

Harbormaster failed remote builds in B22333: Diff 27137!Dec 7 2020, 6:40 PM
sashank planned changes to this revision.Dec 7 2020, 8:53 PM
sashank updated this revision to Diff 27419.Dec 10 2020, 4:35 PM

up

Harbormaster failed remote builds in B22555: Diff 27419!Dec 10 2020, 4:56 PM
catherinewu resigned from this revision.Dec 11 2020, 4:50 AM
sashank updated this revision to Diff 27714.Dec 14 2020, 5:11 PM

rebase past errors

alangenfeld accepted this revision.Dec 14 2020, 5:18 PM
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
Harbormaster completed remote builds in B22787: Diff 27714.Dec 14 2020, 5:34 PM
sashank added inline comments.Dec 14 2020, 5:41 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.

Closed by commit R1:84d5305ac23c: Improve error messages for celery-k8s launcher (authored by sashank). · Explain WhyDec 14 2020, 5:42 PM
This revision was automatically updated to reflect the committed changes.
sashank added a commit: R1:84d5305ac23c: Improve error messages for celery-k8s launcher.

Revision Contents
Changeset List

PathSize
python_modules/
libraries/
dagster-celery-k8s/
dagster_celery_k8s/
52 lines
dagster_celery_k8s_tests/
8 lines

Diff 27723

View Options

python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s/launcher.py

Loading...
View Options

python_modules/libraries/dagster-celery-k8s/dagster_celery_k8s_tests/test_launcher.py

Loading...