Page MenuHomeElementl

[dagster][core] Adding shared QueuedRunCoordinator test suite
ClosedPublic

Authored by sidkmenon on Jun 1 2021, 7:58 PM.
Tags
None
Referenced Files
F2989941: D8187.diff
Thu, Jun 8, 3:52 PM
Unknown Object (File)
Sun, May 14, 12:13 PM
Unknown Object (File)
May 1 2023, 12:33 PM
Unknown Object (File)
Apr 3 2023, 7:12 AM
Unknown Object (File)
Mar 31 2023, 10:58 PM
Unknown Object (File)
Mar 30 2023, 10:45 PM
Unknown Object (File)
Mar 30 2023, 9:05 AM
Unknown Object (File)
Mar 30 2023, 5:52 AM
Subscribers
None

Details

Summary

Adding a shared QueuedRunCoordinator test suite in anticipation for a Run Attribution example (using a Run Coordinator).

D8036 seemed a bit convoluted so going for this simpler approach instead.

Test Plan

bk

Diff Detail

Repository
R1 dagster
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

Looks good to me - although I still largely agree with the blog post I linked on the other diff. https://dagster.phacility.com/D8036#inline-50071

python_modules/dagster/dagster_tests/core_tests/run_coordinator_tests/test_queued_run_coordinator.py
11

Feedback from https://dagster.phacility.com/D8036#inline-50069 still applies - although still a nitpick. Just wanted to make sure you saw it before choosing to abandon the other diff.

This revision is now accepted and ready to land.Jun 1 2021, 8:29 PM

Renaming to QueuedRunCoordinatorTests as per @jordansanders' advice

prefixing with Test to adhere to pytest conventions