added tests to check that 1) success conditions are handled correctly and 2) failure conditions (both within client and from server) are handled gracefully
- R1 dagster
- python-client (branched from master)
No Test Coverage
Adding in automatic inference of repository location and/or repository if not specified
Fixing repo location & repo name inference to work based on the number of unique (repo loc, repo, pipeline) tuples
this should be flattened for readability
spacing, and remove + for concat multiline strings
|21 ↗||(On Diff #35676)|
this is more about the information for a specific a pipeline, rather than the repository or repository location
the extra optional seems to be part of the autogenerated doc strings. but it looks like I accidentally deleted the optionals or something on repository_location_name and repository_name, etc, so I added them back for consistency.
nit: type - weird how this isn't caught by mypy?
Am I understanding correctly that the underlying GraphQL mutation is called launchPipelineExecution? If so, any reason not to call this method launch_pipeline_execution?
yeah that is pretty weird actually.... ?
I think after a conversation with Alex, he recommended calling this submit_pipeline_execution?
I think we have been doing a rename under the hood from launch_pipeline to submit_pipeline, I guess ostensibly because the submit change now sends the run to the daemon which launches it instead. From conversations with Johann, it seems a bit messy, so I'm happy to rename this to launch_pipeline_execution if you think that's better.
for context - when we introduced the RunCoordinator for run queueing we made launch from dagit / graphql actually do submit but didn't follow up on any renames in the product or GraphQL schema
its not obvious whats the best path here, we could
Ah - got it. How difficult would it be to just add submitPipelineExecution to the GraphQL schema?
adding it - pretty easy
Idea - could we land this & then change the backing query on the client when submitPipelineExecution is ready? I have the backwards compat setup already handled so I think we should be good?
Addressing upstream docs comments - also (hopefully) improved error message structure for "InvalidOutputError"
IMO it's worth prioritizing launch -> submit, but definitely not a blocker for this diff.