Differential D5507

Add .join() to grpc server handle cleanup
Needs ReviewPublic
Actions

Authored by dgibson on Dec 9 2020, 5:34 PM.

Details

Reviewers

sashank

Summary

Attempt to get around weird sporadic crashes in BK

Test Plan

BK + pray

Diff Detail

Repository

R1 dagster

Branch

addjoin (branched from master)

Lint

Lint OK

Unit

No Unit Test Coverage

Event Timeline

dgibson created this revision.Dec 9 2020, 5:34 PM

Add a bit more cleanup to mimic what the managed handle does (e.g. if the list repositories call fails - not quite as relevant here sicne we don't need to kill the server, but can't hurt)

Thanks for this, this looks great

python_modules/dagster/dagster/core/host_representation/handle.py
124–126	👍 Good call

test fix

Harbormaster failed remote builds in B22486: Diff 27337!Dec 9 2020, 5:53 PM

Harbormaster failed remote builds in B22487: Diff 27338!

Harbormaster failed remote builds in B22490: Diff 27341!Dec 9 2020, 6:10 PM

dgibson planned changes to this revision.Dec 11 2020, 4:42 AM

dgibson abandoned this revision.Dec 17 2020, 8:57 PM

bringing this back for once the thread-safety issues are fixed in https://dagster.phacility.com/D5790 - since i think it's still a good change to clean up the thread