Moved this diff to do the exact opposite, in https://dagster.phacility.com/D4156

mock DagsterInstance.get() when calling CliRunner functions, so that run_launcher.join() will work

slightly grosser fix to deal with managed grpc servers in a similar way - need to find any handles that the grpc run launcher launched, shut down their server processes, and wait for them to finish (we don't create them in a test context so we can't clean them up there)

yeah I think we concluded at the end of last week that the existing behavior is actually giving us what we want?

up

up

up

py2

Also wait for any grpc server processes to make sure they're all cleaned up before the instance tries to clean up.

test fixes

disable telemetry by default in CLI tests, use a helper method for mocked instances that will persist if you call them in subprocesses

tests are still failing on windows due to the telemetry log

up

one more

more cli tests

add issue #

up

up

up

up

up

up

more test

up

up

https://dagster.phacility.com/D4112 removes the optional DagsterInstance params

up

up

up

up

up

up

feedback

feedback - re:DagsterInstance.get(), my last piece of work for 0.9 task is a big refactor of the test that calls execute_execute_command in a bunch of places, so I'll do that, but roll it into that change coming later today to avoid conflict hell, if that's alright

test tweak

test fix

Add a test that the run will keep going after the API client no longer exists, but will eventually die after that

up

up

up

up

test fix for py2

pasting nick here (who I think largely agrees with you alex) when we talked about this earlier:

don't let grpcserverprocess be a contextmanager anymore - now that it's not managing process lifecycle it doesn't really make sense. Clients that want the server to shut down automatically in a context should be using serverprocess.create_ephemeral_client (or ephemeral_grpc_api_client - so we've sort of come full circle here)

up

test fix

OK, how about this - use an in process repo location when you can, but still be able to execute when you specify a pipeline in a workspace where that is not possible (e.g. a different executable, or a gRPC server)

up

re-attempt test run

test fix attempt

up

up

isort fix

keep the execute command running through user code for now (will send that out separately - left in the infra for it though, like the new method on repositorylocation)

feedback

we could also add a method on RunLauncher that executes the run synchronously? (instead of the repo location, I just put it there since that's where it was before - there's even still a lingering broken callsite in dagster-graphql)