Page MenuHomePhabricator

dgibson (Daniel Gibson)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 6 2020, 12:49 PM (5 w, 1 d)

Recent Activity

Yesterday

dgibson abandoned D4078: Move execute_pipeline to not run any user code.

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

Mon, Aug 10, 8:53 PM
dgibson updated the diff for D4145: Refactor CLI tests to pass in contexts for managed and unmanaged GRPC servers.

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

Mon, Aug 10, 4:30 PM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

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)

Mon, Aug 10, 3:48 PM
dgibson requested review of D4151: Throw from sync_get_external_execution_plan rather than relying on callsites to check error data.
Mon, Aug 10, 3:14 PM
dgibson requested changes to D4122: Even softer termination scheme.

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

Mon, Aug 10, 3:13 PM
dgibson added inline comments to D4087: nicer error on failure to load repository in external process.
Mon, Aug 10, 2:14 PM
dgibson updated the diff for D4145: Refactor CLI tests to pass in contexts for managed and unmanaged GRPC servers.

up

Mon, Aug 10, 1:29 PM
dgibson accepted D4143: Re-enable test_reconstructable.py tests.
Mon, Aug 10, 1:10 PM
dgibson abandoned D4111: Make CLI tests that execute over pipelines include managed and unmanaged GRPC pipelines.
Mon, Aug 10, 1:09 PM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

up

Mon, Aug 10, 1:41 AM

Sun, Aug 9

dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

up

Sun, Aug 9, 1:37 AM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

py2

Sun, Aug 9, 1:00 AM

Sat, Aug 8

dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

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

Sat, Aug 8, 6:49 PM
dgibson planned changes to D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).
Sat, Aug 8, 6:10 PM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

test fixes

Sat, Aug 8, 5:20 PM
dgibson planned changes to D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).
Sat, Aug 8, 2:28 AM

Fri, Aug 7

dgibson updated the summary of D4145: Refactor CLI tests to pass in contexts for managed and unmanaged GRPC servers.
Fri, Aug 7, 11:58 PM
dgibson requested review of D4145: Refactor CLI tests to pass in contexts for managed and unmanaged GRPC servers.
Fri, Aug 7, 11:42 PM
dgibson committed R1:c7396610b677: One more flaky test failure on windows (authored by dgibson).
One more flaky test failure on windows
Fri, Aug 7, 8:55 PM
dgibson closed D4144: One more flaky test failure on windows.
Fri, Aug 7, 8:55 PM
dgibson requested review of D4144: One more flaky test failure on windows.
Fri, Aug 7, 8:40 PM
dgibson requested review of D4138: Replace --host CLI param with --grpc_host to avoid conflicting with dagit --host param.
Fri, Aug 7, 6:14 PM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

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

Fri, Aug 7, 5:48 PM
dgibson committed R1:c7e0ed5f5b2a: Disable tests currently breaking Windows (authored by dgibson).
Disable tests currently breaking Windows
Fri, Aug 7, 5:48 PM
dgibson closed D4135: Disable tests currently breaking Windows.
Fri, Aug 7, 5:48 PM
dgibson planned changes to D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

tests are still failing on windows due to the telemetry log

Fri, Aug 7, 5:38 PM
dgibson requested review of D4137: Add cleanup thread to grpc server.
Fri, Aug 7, 4:39 PM
dgibson updated the diff for D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).

up

Fri, Aug 7, 4:18 PM
dgibson updated the diff for D4135: Disable tests currently breaking Windows.

one more

Fri, Aug 7, 4:17 PM
dgibson requested review of D4136: Re-enable CLI execute/launch/backfill commands on windows (by eliminating sources of tempdir contention).
Fri, Aug 7, 2:54 PM
dgibson updated the diff for D4135: Disable tests currently breaking Windows.

more cli tests

Fri, Aug 7, 2:25 PM
dgibson updated the diff for D4135: Disable tests currently breaking Windows.

add issue #

Fri, Aug 7, 2:19 PM
dgibson requested review of D4135: Disable tests currently breaking Windows.
Fri, Aug 7, 2:17 PM

Thu, Aug 6

dgibson planned changes to D4111: Make CLI tests that execute over pipelines include managed and unmanaged GRPC pipelines.
Thu, Aug 6, 2:23 AM
dgibson committed R1:1ebb0674f970: Don't kill the grpc server process when the parent process ends (authored by dgibson).
Don't kill the grpc server process when the parent process ends
Thu, Aug 6, 2:07 AM
dgibson closed D4096: Don't kill the grpc server process when the parent process ends.
Thu, Aug 6, 2:07 AM
dgibson committed R1:6e9741345a17: Split test_cli_commands.py into its own BK test (authored by dgibson).
Split test_cli_commands.py into its own BK test
Thu, Aug 6, 2:06 AM
dgibson closed D4083: Split test_cli_commands.py into its own BK test.
Thu, Aug 6, 2:06 AM
dgibson committed R1:2351e4d7e708: Move schedule backfill command to use External classes instead of user code… (authored by dgibson).
Move schedule backfill command to use External classes instead of user code…
Thu, Aug 6, 1:44 AM
dgibson closed D4071: Move schedule backfill command to use External classes instead of user code classes.
Thu, Aug 6, 1:44 AM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

up

Thu, Aug 6, 1:21 AM
dgibson updated the diff for D4083: Split test_cli_commands.py into its own BK test.

up

Thu, Aug 6, 1:20 AM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Thu, Aug 6, 1:19 AM
dgibson updated the diff for D4111: Make CLI tests that execute over pipelines include managed and unmanaged GRPC pipelines.

up

Thu, Aug 6, 12:44 AM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

up

Thu, Aug 6, 12:43 AM
dgibson updated the diff for D4083: Split test_cli_commands.py into its own BK test.

up

Thu, Aug 6, 12:41 AM
dgibson committed R1:1473afba1527: Move launch_pipeline CLI commands to not run any user code (authored by dgibson).
Move launch_pipeline CLI commands to not run any user code
Thu, Aug 6, 12:38 AM
dgibson closed D4061: Move launch_pipeline CLI commands to not run any user code.
Thu, Aug 6, 12:38 AM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

more test

Thu, Aug 6, 12:35 AM

Wed, Aug 5

dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Wed, Aug 5, 11:55 PM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

up

Wed, Aug 5, 11:54 PM
dgibson accepted D4099: Heartbeat for GRPC server.
Wed, Aug 5, 11:39 PM
dgibson published D4112: Remove optional DagsterInstance param from execute CLI path for review.
Wed, Aug 5, 9:30 PM
dgibson requested review of D4111: Make CLI tests that execute over pipelines include managed and unmanaged GRPC pipelines.
Wed, Aug 5, 9:29 PM
dgibson added a comment to D4061: Move launch_pipeline CLI commands to not run any user code.

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

Wed, Aug 5, 9:29 PM
dgibson removed a reviewer for D4061: Move launch_pipeline CLI commands to not run any user code: alangenfeld.
Wed, Aug 5, 9:28 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

up

Wed, Aug 5, 8:00 PM
dgibson updated the diff for D4083: Split test_cli_commands.py into its own BK test.

up

Wed, Aug 5, 7:59 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Wed, Aug 5, 7:58 PM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

up

Wed, Aug 5, 7:57 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

up

Wed, Aug 5, 6:00 PM
dgibson updated the diff for D4083: Split test_cli_commands.py into its own BK test.

up

Wed, Aug 5, 5:59 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

feedback

Wed, Aug 5, 5:58 PM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

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

Wed, Aug 5, 4:59 PM
dgibson added inline comments to D4061: Move launch_pipeline CLI commands to not run any user code.
Wed, Aug 5, 4:44 PM
dgibson added inline comments to D4061: Move launch_pipeline CLI commands to not run any user code.
Wed, Aug 5, 4:37 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

test tweak

Wed, Aug 5, 3:30 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

test fix

Wed, Aug 5, 3:09 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

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

Wed, Aug 5, 2:56 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

up

Wed, Aug 5, 2:04 PM
dgibson updated the diff for D4083: Split test_cli_commands.py into its own BK test.

up

Wed, Aug 5, 1:58 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Wed, Aug 5, 1:57 PM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

up

Wed, Aug 5, 1:56 PM
dgibson planned changes to D4078: Move execute_pipeline to not run any user code.
Wed, Aug 5, 1:53 PM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

test fix for py2

Wed, Aug 5, 4:29 AM
dgibson added a comment to D4078: Move execute_pipeline to not run any user code.

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

Wed, Aug 5, 4:20 AM
dgibson updated the diff for D4096: Don't kill the grpc server process when the parent process ends.

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)

Wed, Aug 5, 3:53 AM

Tue, Aug 4

dgibson added reviewers for D4071: Move schedule backfill command to use External classes instead of user code classes: catherinewu, schrockn.
Tue, Aug 4, 8:33 PM
dgibson requested review of D4096: Don't kill the grpc server process when the parent process ends.
Tue, Aug 4, 8:32 PM
dgibson added a reviewer for D4083: Split test_cli_commands.py into its own BK test: max.
Tue, Aug 4, 8:15 PM
dgibson requested review of D4083: Split test_cli_commands.py into its own BK test.
Tue, Aug 4, 8:15 PM
dgibson added inline comments to D4078: Move execute_pipeline to not run any user code.
Tue, Aug 4, 4:39 PM
dgibson updated the diff for D4078: Move execute_pipeline to not run any user code.

up

Tue, Aug 4, 4:01 PM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

test fix

Tue, Aug 4, 4:00 PM
dgibson updated the diff for D4078: Move execute_pipeline to not run any user code.

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)

Tue, Aug 4, 3:22 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Tue, Aug 4, 2:18 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

re-attempt test run

Tue, Aug 4, 1:40 PM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

test fix attempt

Tue, Aug 4, 1:34 PM
dgibson requested review of D4078: Move execute_pipeline to not run any user code.
Tue, Aug 4, 2:12 AM
dgibson accepted D4063: remove flytekit pin.
Tue, Aug 4, 1:16 AM
dgibson committed R1:c25c07de0e92: Make schedule launch command work with grpc server origins (authored by dgibson).
Make schedule launch command work with grpc server origins
Tue, Aug 4, 1:16 AM
dgibson closed D4058: Make schedule launch command work with grpc server origins.
Tue, Aug 4, 1:16 AM
dgibson updated the diff for D4071: Move schedule backfill command to use External classes instead of user code classes.

up

Tue, Aug 4, 1:05 AM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

up

Tue, Aug 4, 1:03 AM
dgibson updated the diff for D4058: Make schedule launch command work with grpc server origins.

isort fix

Tue, Aug 4, 12:58 AM
dgibson retitled D4061: Move launch_pipeline CLI commands to not run any user code from Move execute_pipeline and launch_pipeline CLI commands to not run any user code to Move launch_pipeline CLI commands to not run any user code.
Tue, Aug 4, 12:54 AM
dgibson updated the diff for D4061: Move launch_pipeline CLI commands to not run any user code.

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)

Tue, Aug 4, 12:51 AM
dgibson updated the diff for D4058: Make schedule launch command work with grpc server origins.

feedback

Tue, Aug 4, 12:45 AM

Mon, Aug 3

dgibson added a comment to D4061: Move launch_pipeline CLI commands to not run any user code.

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)

Mon, Aug 3, 9:31 PM
dgibson requested review of D4071: Move schedule backfill command to use External classes instead of user code classes.
Mon, Aug 3, 9:26 PM