Page MenuHomeElementl
Differential D8187

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

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
View All Files
Subscribers
None

Details

Reviewers
• jordansanders
• johann
Commits
R1:9bc05f8fd0f3: [dagster][core] Adding shared QueuedRunCoordinator test suite
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

• sidkmenon created this revision.Jun 1 2021, 7:58 PM
• sidkmenon added a child revision: D8043: [examples] Adding a Run Attribution Example via a Custom Run Coordinator.Jun 1 2021, 7:58 PM
Harbormaster completed remote builds in B31481: Diff 38743.Jun 1 2021, 8:22 PM
• sidkmenon requested review of this revision.Jun 1 2021, 8:22 PM
• jordansanders accepted this revision.Jun 1 2021, 8:29 PM

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–72

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
• sidkmenon updated this revision to Diff 38750.Jun 1 2021, 8:42 PM

Renaming to QueuedRunCoordinatorTests as per @jordansanders' advice

Harbormaster completed remote builds in B31488: Diff 38750.Jun 1 2021, 9:02 PM
• sidkmenon updated this revision to Diff 38804.Jun 2 2021, 5:48 PM

prefixing with Test to adhere to pytest conventions

Harbormaster completed remote builds in B31528: Diff 38804.Jun 2 2021, 6:11 PM
Closed by commit R1:9bc05f8fd0f3: [dagster][core] Adding shared QueuedRunCoordinator test suite (authored by Sidharth Menon <menon.sid.k@gmail.com>). · Explain WhyJun 3 2021, 6:55 PM
This revision was automatically updated to reflect the committed changes.
Sidharth Menon <menon.sid.k@gmail.com> added a commit: R1:9bc05f8fd0f3: [dagster][core] Adding shared QueuedRunCoordinator test suite.

Revision Contents
Changeset List

PathSize
python_modules/
dagster/
dagster_tests/
core_tests/
run_coordinator_tests/
214 lines

Diff 38804

View Options

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

Loading...