Page MenuHomePhabricator

[RFC] Add util to execute remote pipeline runs
ClosedPublic

Authored by sashank on Jan 24 2020, 1:33 AM.

Details

Summary
from dagster_graphql.client.execute import execute_remote_pipeline_run

res = execute_remote_pipeline_run(
    host="https://your-remote-dagster.com/",
    pipeline_name="hello_world_pipeline",
    environment_dict={},
)
Test Plan

unit

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sashank created this revision.Jan 24 2020, 1:33 AM
sashank edited the summary of this revision. (Show Details)Jan 24 2020, 1:34 AM
sashank edited the summary of this revision. (Show Details)
sashank retitled this revision from [RFC] Add util to execute remote pipelie runs to [RFC] Add util to execute remote pipeline runs.Jan 24 2020, 1:39 AM
schrockn requested changes to this revision.Jan 24 2020, 1:41 AM

this seems good. let's add a test for it (at minimum mock out something) just for code coverage so we can catch syntax errors or a variable name or something.

python_modules/dagster-graphql/dagster_graphql/client/execute.py
13

does mode=None not work? would rather than have the "default" string all over the codebase

25

what does this end up returning?

This revision now requires changes to proceed.Jan 24 2020, 1:41 AM
sashank updated this revision to Diff 8904.Jan 24 2020, 11:19 PM
sashank edited the summary of this revision. (Show Details)
  • Add checks
  • Add test
python_modules/dagster-graphql/dagster_graphql/client/execute.py
13

ExecutionParams expects a non-None value. Updated this to use DEFAULT_MODE_NAME like the rest of the codebase instead of hardcoding "default"

25

Returns the gql response for START_PIPELINE_EXECUTION_MUTATION as a dict. For example:

{
    'data': {
        'startPipelineExecution': {
            '__typename': 'StartPipelineExecutionSuccess',
            'run': {
                'runId': '41434376-f22c-4173-93ba-630eee54545b',
                'status': 'NOT_STARTED',
                'pipeline': {'name': 'hello_world_pipeline'},
                'logs': {
                    'nodes': [{'__typename': 'PipelineProcessStartEvent'}],
                    'pageInfo': {
                        'lastCursor': 0,
                        'hasNextPage': None,
                        'hasPreviousPage': None,
                        'count': 1,
                        'totalCount': 1,
                    },
                },
                'environmentConfigYaml': '{}\n',
                'mode': 'default',
            },
        }
    }
}
sashank updated this revision to Diff 8907.Jan 24 2020, 11:22 PM

Move print out of execute_query_against_remote

sashank edited the test plan for this revision. (Show Details)Jan 24 2020, 11:28 PM
sashank added a reviewer: alangenfeld.
sashank updated this revision to Diff 8908.Jan 24 2020, 11:36 PM

Add mock to dagster-graphql setup.py

schrockn requested changes to this revision.Jan 24 2020, 11:42 PM

one last change on the mock

python_modules/dagster-graphql/dagster_graphql/client/execute.py
26

ok got it

python_modules/dagster-graphql/dagster_graphql_tests/client_tests/test_execute.py
6

i don't really see a need to mock out the return value fully. it could just be a sentinel value. this test could get misleading if the return schema for startPipelineExecution changes. either it will drift or people will do unnecessary work to "fix" it

python_modules/dagster-graphql/setup.py
41 ↗(On Diff #8908)

superfluous commas

This revision now requires changes to proceed.Jan 24 2020, 11:42 PM
sashank updated this revision to Diff 8909.Jan 24 2020, 11:48 PM

Correctly add pytest-mock

sashank added inline comments.Jan 24 2020, 11:49 PM
python_modules/dagster-graphql/dagster_graphql_tests/client_tests/test_execute.py
6

great call out - fixed

python_modules/dagster-graphql/setup.py
41 ↗(On Diff #8908)

this is moved to dev-requirements.txt

sashank updated this revision to Diff 8910.Jan 24 2020, 11:50 PM

Remove extra fields from mocked value

Harbormaster failed remote builds in B7232: Diff 8910!
schrockn accepted this revision.Jan 25 2020, 12:11 AM
This revision is now accepted and ready to land.Jan 25 2020, 12:11 AM