Page MenuHomeElementl

fix cleanupr issues in scheduler tests
ClosedPublic

Authored by dgibson on Oct 20 2020, 6:50 PM.

Details

Summary

we need to join the run launchers when the test opens a subprocess and passes in an instance ref.

Test Plan

Scheduler tests still pass, hopefully are no longer flaky

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dgibson published this revision for review.Oct 20 2020, 6:53 PM
python_modules/dagster/dagster_tests/scheduler_tests/test_scheduler_run.py
232–241 ↗(On Diff #24099)

its not clear to me this would fix it, whats your thinking?

hmmm maybe not. I assumed it was related to https://dagster.phacility.com/D4823 where I made this change

new fix coming

python_modules/dagster/dagster_tests/scheduler_tests/test_scheduler_run.py
193–212 ↗(On Diff #24099)

ya seems like it should work, any subprocesses in play that may not be getting cleaned up?

dgibson retitled this revision from fix contextmanager issues in scheduler tests to fix cleanupr issues in scheduler tests.
dgibson edited the summary of this revision. (Show Details)

here is a more plausible fix, I guess that diff landing just before the failure was a coincidence

This revision is now accepted and ready to land.Oct 20 2020, 7:36 PM
This revision was automatically updated to reflect the committed changes.