[dagster cli] Add option to dagster definitions validate to load in-process

[benpankow]

Summary

By default, dagster definitions validate starts up a gRPC server to load the code location. This adds a decent amount of runtime to the command (locally, for me, about 1s of a total 2.6s runtime).

This PR instead defaults to loading the code location in-process, unless pointing at multiple code locations (via workspace.yaml) or a location which specifies an executable_path. This makes the command pretty quick, with the runtime almost entirely dominated by importing the user's code and loading the defintions (around 1.6s for a full components project for me).

How I Tested These Changes

Update unit tests.

Changelog

By default, dagster definitions validate command now loads locations in-process, which speeds up runtime.

[benpankow]

• [dagster cli] Add option to dagster definitions validate to load in-process #27459 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[smackesey]

Is there any reason not to just always load them in-process?

[benpankow]

Is there any reason not to just always load them in-process?

I think the main case in-process can't handle is validating multiple code locations w/ potentially different Python environments. The other thing is that locations can specify a python executable, that this won't address. Should explicitly test these cases & disallow the flag if they're present.

[smackesey]

I think the main case in-process can't handle is validating multiple code locations w/ potentially different Python environments. The other thing is that locations can specify a python executable, that this won't address. Should explicitly test these cases & disallow the flag if they're present.

I see-- this is good but I have a strong suspicion most users won't benefit if it's not the default behavior. Also it seems like if different code locations have different python environments, we still shouldn't need to start a code server-- couldn't we just resolve the python executable and run an in-process validation using the resolved executable? Seems like it would simplify things.

[benpankow]

Updated to default to loading in-process, with the above exceptions.

Tagging @alangenfeld and @gibsondan since they might have a better idea if there are other nuances to be wary of here.

[smackesey]

If we have this, do we even need to give the users the --load-with-grpc option? IIUC we can just detect which method to use and fall back to grpc when necessary. If we do it this way then maybe we can eliminate it entirely later.

Just trying to avoid adding API surface.

[benpankow]

Yeah, that's fair. I figured it's sort of nice to have the flag to force consistently using gRPC if for some reason a user wants to make sure the command does the same thing each time, but don't feel strongly.

[smackesey]

IMO the method used to load the definitions should be treated as an implementation detail. If there are errors that would somehow only be found from gRPC startup, I would think that those errors are outside the scope of the commands expected output.

Maybe I'm wrong though, will let others weigh in.

[schrockn]

Requesting changes to resolve discussion raised by @smackesey. My inclination is to remove the grpc server option entirely as well, but would like @maximearmstrong and @gibsondan to weigh in