Page MenuHomePhabricator

Basic implementation of priority for Celery executor (single-queue)
ClosedPublic

Authored by max on Dec 18 2019, 12:33 AM.

Details

Summary

Allows numeric priorities from 10 (highest) to 0 (lowest), set as metadata keys on the solid.

Test Plan

Manual

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

max created this revision.Dec 18 2019, 12:33 AM
max updated this revision to Diff 7923.Dec 18 2019, 1:23 AM

Rebase

max updated this revision to Diff 7926.Dec 18 2019, 1:33 AM

Rebase

schrockn requested changes to this revision.Dec 18 2019, 8:47 PM

do we

js_modules/dagit/src/plan/ExecutionPlanBox.tsx
150 ↗(On Diff #7912)

unrelated?

python_modules/dagster-celery/dagster_celery/engine.py
102

what does this mean?

python_modules/dagster-celery/dagster_celery_tests/test_priority.py
24

i really do think we should have this under ci before we merge this to master

61

can we verify that the metadata exists here?

This revision now requires changes to proceed.Dec 18 2019, 8:47 PM
max added inline comments.Dec 19 2019, 2:23 AM
js_modules/dagit/src/plan/ExecutionPlanBox.tsx
150 ↗(On Diff #7912)
python_modules/dagster-celery/dagster_celery/engine.py
102

i'll flesh it out

python_modules/dagster-celery/dagster_celery_tests/test_priority.py
24

i can run this eagerly in CI, but that won't test the priority handling (there is no queue when running eagerly). i do not want to block merging this on figuring out buildkite networking.

61

i suppose, but i don't understand what that tests

I guess this is related to the previous conversation where I think we should figure out a way to run celery as a library we some sort of simple queue so we don't need to do really heavy infrastructure to test things like the queueing

max updated this revision to Diff 7985.Dec 19 2019, 2:41 AM

Rebase

Harbormaster failed remote builds in B6459: Diff 7983!
schrockn accepted this revision.Dec 19 2019, 2:48 AM
This revision is now accepted and ready to land.Dec 19 2019, 2:48 AM
max updated this revision to Diff 8007.Dec 19 2019, 4:39 PM

Fixup