Update forked repository. #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary:
Example output:
```
{"__class__": "ExecutePipelineStartMessage"}
{"__class__": "DagsterEvent", "event_specific_data": null, "event_type_value": "PIPELINE_START", "logging_tags": {}, "message": "Started execution of pipeline \"test_pipeline\".", "pipeline_name": "test_pipeline", "solid_handle": null, "step_key": null, "step_kind_value": null}
{"__class__": "DagsterEvent", "event_specific_data": {"__class__": "EngineEventData", "error": null, "marker_end": null, "marker_start": null, "metadata_entries": [{"__class__": "EventMetadataEntry", "description": null, "entry_data": {"__class__": "TextMetadataEntryData", "text": "35573"}, "label": "pid"}, {"__class__": "EventMetadataEntry", "description": null, "entry_data": {"__class__": "TextMetadataEntryData", "text": "['test_solid.compute']"}, "label": "step_keys"}]}, "event_type_value": "ENGINE_EVENT", "logging_tags": {}, "message": "Executing steps in process (pid: 35573)", "pipeline_name": "test_pipeline", "solid_handle": null, "step_key": null, "step_kind_value": null}
{"__class__": "DagsterEvent", "event_specific_data": null, "event_type_value": "STEP_START", "logging_tags": {"pipeline": "test_pipeline", "solid": "test_solid", "solid_definition": "test_solid", "step_key": "test_solid.compute"}, "message": "Started execution of step \"test_solid.compute\".", "pipeline_name": "test_pipeline", "solid_handle": {"__class__": "SolidHandle", "name": "test_solid", "parent": null}, "step_key": "test_solid.compute", "step_kind_value": "COMPUTE"}
{"__class__": "DagsterEvent", "event_specific_data": {"__class__": "StepOutputData", "intermediate_materialization": null, "step_output_handle": {"__class__": "StepOutputHandle", "output_name": "result", "step_key": "test_solid.compute"}, "type_check_data": {"__class__": "TypeCheckData", "description": null, "label": "result", "metadata_entries": [], "success": true}}, "event_type_value": "STEP_OUTPUT", "logging_tags": {"pipeline": "test_pipeline", "solid": "test_solid", "solid_definition": "test_solid", "step_key": "test_solid.compute"}, "message": "Yielded output \"result\" of type \"Any\". (Type check passed).", "pipeline_name": "test_pipeline", "solid_handle": {"__class__": "SolidHandle", "name": "test_solid", "parent": null}, "step_key": "test_solid.compute", "step_kind_value": "COMPUTE"}
{"__class__": "DagsterEvent", "event_specific_data": {"__class__": "StepSuccessData", "duration_ms": 6.887455000000209}, "event_type_value": "STEP_SUCCESS", "logging_tags": {"pipeline": "test_pipeline", "solid": "test_solid", "solid_definition": "test_solid", "step_key": "test_solid.compute"}, "message": "Finished execution of step \"test_solid.compute\" in 6.89ms.", "pipeline_name": "test_pipeline", "solid_handle": {"__class__": "SolidHandle", "name": "test_solid", "parent": null}, "step_key": "test_solid.compute", "step_kind_value": "COMPUTE"}
{"__class__": "DagsterEvent", "event_specific_data": {"__class__": "EngineEventData", "error": null, "marker_end": null, "marker_start": null, "metadata_entries": [{"__class__": "EventMetadataEntry", "description": null, "entry_data": {"__class__": "TextMetadataEntryData", "text": "35573"}, "label": "pid"}, {"__class__": "EventMetadataEntry", "description": null, "entry_data": {"__class__": "TextMetadataEntryData", "text": "['test_solid.compute']"}, "label": "step_keys"}]}, "event_type_value": "ENGINE_EVENT", "logging_tags": {}, "message": "Finished steps in process (pid: 35573) in 23ms", "pipeline_name": "test_pipeline", "solid_handle": null, "step_key": null, "step_kind_value": null}
{"__class__": "DagsterEvent", "event_specific_data": null, "event_type_value": "PIPELINE_SUCCESS", "logging_tags": {}, "message": "Finished execution of pipeline \"test_pipeline\".", "pipeline_name": "test_pipeline", "solid_handle": null, "step_key": null, "step_kind_value": null}
{"__class__": "ExecutePipelineEndMessage"}
```
Example output in case of framework error:
```
{"__class__": "ExecutePipelineStartMessage"}
{"__class__": "SerializableErrorInfo", "cause": null, "cls_name": "TypeError", "message": "TypeError: the JSON object must be str, bytes or bytearray, not NoneType\n", "stack": [" File \"/Users/sashankthupukari/projects/dagster/python_modules/dagster/dagster/cli/api.py\", line 106, in execute_pipeline_command\n environment_dict = json.loads(environment_dict)\n", " File \"/Users/sashankthupukari/.pyenv/versions/3.7.5/lib/python3.7/json/__init__.py\", line 341, in loads\n raise TypeError(f'the JSON object must be str, bytes or bytearray, '\n"]}
{"__class__": "ExecutePipelineEndMessage"}
```
Test Plan: unit
Reviewers: prha, schrockn, alangenfeld
Reviewed By: schrockn
Differential Revision: https://dagster.phacility.com/D2806
Test Plan: manual Reviewers: schrockn Differential Revision: https://dagster.phacility.com/D2970
Test Plan: unit Reviewers: schrockn Differential Revision: https://dagster.phacility.com/D2971
Summary: Spell checker continuing to pay dividends Test Plan: BK Reviewers: max, alangenfeld, sashank Reviewed By: sashank Differential Revision: https://dagster.phacility.com/D2975
Summary: This diff removes the repository definition from the schedule execution context. This was initially added as a hack for the backfilling scheduler, but is unecessary because schedule definitions and repository definitions are co-located now. This lets use remove a few more calls to `get_repository_definition()` in `dagster-graphql` Test Plan: unit Reviewers: schrockn, alangenfeld Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2972
…ository Summary: Simply getting rid of a few more calls to `get_repository_definition` where it is unecessary Test Plan: unit Reviewers: schrockn, alangenfeld Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2973
Summary: This is to support run group-oriented UI Test Plan: Unit Reviewers: yuhan, alangenfeld, prha, bengotow Reviewed By: yuhan Subscribers: bengotow, prha Differential Revision: https://dagster.phacility.com/D2444
Summary: Stacked on D2444 Test Plan: Unit Reviewers: prha, bengotow, alangenfeld, schrockn, yuhan Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2470
Summary:
Inform the user when a run cannot be reexecuted because the pipeline is not present in the current repository.
{F140800}
Test Plan: Manual
Reviewers: yuhan, prha
Reviewed By: yuhan, prha
Differential Revision: https://dagster.phacility.com/D2969
Summary: A lot of these were referring to the wrong issue number Test Plan: BK Reviewers: sashank Reviewed By: sashank Differential Revision: https://dagster.phacility.com/D2977
Summary: #2453 a few changes - untie the step selection from log filter * code-wise, `ReexecuteWithData` now owns - `selectedSteps` which was previously generated from `Run`'s state `logsFilter` - `query` which was previously owned by `GaantChart`. We lifted it up b/c click event on single step will also update query now. * behavior-wise, click on a step will 1) select a step, 2) filter the log of this step, and 3) update the selector query. Removing the filter won't deselect the step. - click a selected step will deselect it, remove it from log filter, and clear the selector query - the "type a step subset" input will now change the `selectedSteps` therefore enable multi-step re-execution * the step selector no longer hide unselected steps by default. we now have a checkbox to give users the option to show/hide unselected steps single step selection via single-click {F139989} arbitrary step subset selection via selector syntax {F139980} Test Plan: click around dagit - playground - run Reviewers: bengotow, schrockn, max Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2959
Summary: #2471 Test Plan: bk {F140971} Reviewers: max, bengotow Reviewed By: max Differential Revision: https://dagster.phacility.com/D2981
Summary: Fixes broken tests on windows Test Plan: unit Reviewers: max, prha, alangenfeld Reviewed By: alangenfeld Subscribers: alangenfeld Differential Revision: https://dagster.phacility.com/D2983
Summary: Spelling mistakes are bad. Also for avoiding keyword collisions standard practice is to append underscores Test Plan: BK Reviewers: alangenfeld, max, sashank Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D2985
Summary: Remove back compat support for old schedules and paritions loading scheme. To be landed for 0.8.0 release Test Plan: bk Reviewers: prha, sashank Reviewed By: sashank Subscribers: schrockn Differential Revision: https://dagster.phacility.com/D2709
… dagster cli as api Summary: This adds a place to collect calls over the dagster cli api. Adapts test code written to test as a top level API. Dagster-graphql and other host processes invoke these directly. This would also be the layer where we replace the implementation with grpc (or similar) when that becomes appropriate. Test Plan: BK Reviewers: max, sashank, alangenfeld Reviewed By: sashank Differential Revision: https://dagster.phacility.com/D2978
Summary: We need this in order for a host process and a user process to share instance information This PR also adds an error message to the IPC channel. I had difficultly debugging a JSON parsing error and being able to send an error object across with a custom message was much easier that the bare serializable error info. This also distinguishes between "user" errors and protocol errors in this layer. Depends on D2978 Test Plan: BK Reviewers: max, alangenfeld, sashank Reviewed By: sashank Differential Revision: https://dagster.phacility.com/D2980
…er.core.codepointer Summary: I want to refer to these classes in ExternalRepository, so it can remember where it came from and marshal that information back and forth to user processes. I do not want the data structures in dagster.core.host_representation to directly container anything in dagster.core.definitions, so moving this. Test Plan: BK Reviewers: max, alangenfeld Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D2987
Summary: Pipeline CLI instructions were missing group (ie 'pipeline') Test Plan: ran locally. Reviewers: prha Reviewed By: prha Differential Revision: https://dagster.phacility.com/D2966
Summary:
Restructure `DagsterGraphQLContext` to be aware of 1. the instance and 2. a collection of `DagsterEnvironment`s
`DagsterEnvironment` is an interface that will allow us to move user code invocations out of the `dagster-graphql` process in to a separate `dagster` process. During migration we maintain `InProcessDagsterEnvironment` for satisfying the API with user code in process.
Since single env single repo -> multi env multi repo is a fundamental change for dagster-graphql & dagit to handle - we introduce back compatability with a collection of `legacy_` prefixed apis that assume one in process environment and one repo in that environment.
The stack of diffs above this will slowly remove all of the `legacy_` functions and push the env & repo selection upwards til eventually `dagit` is aware and the single-repo assumption is gone.
{F141108}
Test Plan: buildkite
Reviewers: schrockn, max, sashank
Reviewed By: schrockn
Differential Revision: https://dagster.phacility.com/D2988
Summary: 🤦 left a `console.log` in D2959 Test Plan: bk Reviewers: max, schrockn Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2982
Summary:
This diff replaces the Dagit sidebar with one that clearly separates "instance"-scoped concepts (assets, schedules) from "environment" scoped pipelines and solids. The new design is also more pipeline-centric and shows the environments pipelines directly in the sidebar. We may end up collapsing this back into a "narrow" bar or having it slide out on hover, etc., but we wanted to establish the new design first and then revisit whether it'd be possible to shrink.
This is the first of a couple major changes we want to make to the Dagit UI to reflect the new pipeline focus - the plan is to swap out the "Definition / Playground" options in the sidebar (sort of a sub-selection right now) for a set of top tabs, but that change involves updating a lot of routes / URLs and I think it'll be easier to review separately.
Todo Soon:
- Where should environment name come from? Need a small GraphQL API for this until we have multi-env support? (OR: is there an agreeable default we can use for now?)
Lower priority improvements up for grabs:
- Implement keyboard up/down + enter in the inline search box
- Make the left nav column resizable by dragging the right edge
- Unify the empty states of the Runs / Schedules / Assets pages
+ Runs has header, schedules doesn't
+ Assets background is a different color, centered content is further down
{F140554}
Test Plan: Update snapshot tests
Reviewers: alangenfeld, schrockn, prha
Reviewed By: prha
Differential Revision: https://dagster.phacility.com/D2967
Summary: Handle the possibility of concurrent RunStorage add_run() by trying to fetch the run again after DagsterRunAlreadyExists #2412 Test Plan: todo Reviewers: max, schrockn, sashank Reviewed By: schrockn Subscribers: schrockn, sashank, max Differential Revision: https://dagster.phacility.com/D2719
Summary: good night sweet prince Completes the deprecation process for `ExecutionSelector` and introduces a `dagster-graphql` only concept of `PipelineSelector` Test Plan: buildkite Reviewers: max, prha, schrockn Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D2995
Test Plan: Rendered run action menu on runs page Reviewers: max, yuhan Reviewed By: yuhan Differential Revision: https://dagster.phacility.com/D2998
Summary: I will likely be adding new storage for run-process lookup table and I didn't want to copy-paste this. Test Plan: BK Reviewers: max, alangenfeld Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D2992
Summary: As we write more small examples like `dep_dsl`, We will eventually deprecate `dagster_examples` Test Plan: Ran the gitpod task command ``` cd examples/$EXAMPLE pip3 install -r requirements.txt [ -e README.md ] && gp open README.md clear && dagit ``` Reviewers: sashank, schrockn Reviewed By: schrockn Differential Revision: https://dagster.phacility.com/D3002
Summary: Adds a simple command to scaffold out an example Test Plan: Run `python bin/create_example.py` Reviewers: alangenfeld, schrockn, yuhan Reviewed By: yuhan Differential Revision: https://dagster.phacility.com/D3005
Summary: {F141658}
Test Plan: bk
Reviewers: sashank, schrockn
Reviewed By: sashank
Differential Revision: https://dagster.phacility.com/D3001
Summary: Same diff, but for master Test Plan: unit Reviewers: alangenfeld, schrockn Reviewed By: schrockn Subscribers: schrockn Differential Revision: https://dagster.phacility.com/D2991
Summary: Personal preference, but if there is enough graphql in the file the autoindent goes haywire. Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3073
Summary: This diff adds graphql_context_test_suite.py and its workhorse function make_graphql_context_test_suite. It should serve as a nice base to drive our large architectural migration leading up to 0.8.0, as well as foundation for an entire suite of acceptance tests for different deployments of the system. This test suite has a number of overall goals: - Have the ability to specify tests that run against multiple instances of the graphql contexts. - Specify which graphql contexts are created in the test file, not a centralized fixture file. - Manage the lifecycle of those graphql contexts and their constituent components (e.g. DagsterInstance, DagsterEnvironment, and so forth) - Have “off-the-shelf” contexts and sets of contexts for common configurations, but with the ability to provide test-specific ones as well. - Organize all the tests in dagster-graphql by providing sensible markers to all the common configurations, so we can easily say “run all the tests that use Sqlite” or “run all the tests that use the synchronous execution manager” or “run all the tests are are purely in-memory”. With this diff you can now run `pytest python_modules/dagster-graphql/dagster_graphql_tests/ -m in_memory` and run only the tests governed by this test suite that have been automatically marked as in_meory. This will allow us to manage this better during local development (faster tests) as well as chop up this test suite into many parallel CI runs. - Provide a straightforward path to run these tests against other deployment configurations, e.g. against a postgres database or running on a k8s cluster. We should build this towards being an acceptance test suite for different deployments of dagster. All tests managed by this suite are marked with graphql_context_test_suite. This will make it straightforward to run all tests not managed by this suite when we start chopping up this test suite for CI. For an example of creating a custom variant, see https://dagster.phacility.com/D3091 Specifically this was designed to help manage our major re-architecture leading up to 0.8.0. They are as follows 1) Launch-Start Consolidation 2) Dagit as Host Process 3) Replacement of PipelineExecutionManager with CliApiRunLauncher (related to 1) 4) User code container This new test suite structure allows these different re-architectures to be largely parallelized. As someone is implementing one of those workstreams, they add a new GraphQLContextVariant type to tests one-by-one. 1) Launch Start Consolidation: We are unifying launch and start. In order to do this we are implementing a synchronous in memory launcher. This will also allow us to have a "fast path" for tests that touch disk infrequently and launch no processes. This works by having the run launch declare that it is "hijacking" the start process. The diff sets up this work stream: https://dagster.phacility.com/D3080 2) Dagit as Host Process: This changes dagit to load repository metadata from the cli api, as opposed to loading definitions directly. This diff sets up that workstream: https://dagster.phacility.com/D3083 3) SubprocessExecutionManager to CliApiRunLauncher: This moves tests that used to use the SubprocessExecutionManager to instead go through the launch codepaths and use the new CliApiRunLauncer. One this and the "Dagit as Host Process" workstream is the complete we will be able to use dagit as a host process exclusively. And once 1, 2, and 3 are complete it should be completely safe to delete the start codepaths all the way down to dagit. Diff that sets up this workstream: https://dagster.phacility.com/D3081 4) User Code in a container. Not set up yet, but this would involve creating a new DagsterEnvironment type that knows how to talk to container. Challenges here are getting a reliable channel between the container and the host process (the OutOfProcessDagsterEnviroment just uses the filesystem) Depends on D3073 Test Plan: BK Reviewers: max, alangenfeld, prha, sashank Reviewed By: alangenfeld Subscribers: alangenfeld Differential Revision: https://dagster.phacility.com/D3074
Summary: Prep for multirepo world Test Plan: Unit Reviewers: alangenfeld, schrockn Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3059
Summary:
This currently implements type system support for scalar types on inputs and outputs. This also adds a dependency on a metaprogramming library called forge, that feels a bit out of date. However, this will get us to a demo faster I believe.
This executes multi-step DAGs in Flyte successfully and captures the inputs and output results.
{F145645}
{F145652}
Test Plan:
First, run Flyte
1. $ cd python_modules/libraries/dagster-flyte/examples
2. $ make docker_build
3. $ docker run --network host -e FLYTE_PLATFORM_URL='127.0.0.1:30081' {{ your docker image }} pyflyte -p dagstertest -d development -c sandbox.config register workflows
Reviewers: schrockn, nate, alangenfeld
Reviewed By: schrockn
Differential Revision: https://dagster.phacility.com/D2901
…aphql context test suite Summary: This provides an example of a test that only makes sense with an asynchronous execution manager, so it takes a different context variants than the previous diffs Depends on D3075 Test Plan: BK Reviewers: alangenfeld, max, prha Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3076
…peline to be a graphql context test suite Summary: This makes it so that these tests are run on three legacy context variants (legacy being defined by use of the legacy execution manager). Depends on D3076 Test Plan: BK Reviewers: max, alangenfeld, prha Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3077
Summary: We are going to use this to consolidate the start and launch code paths, and to have an in-memory implementation of a launch for fast tests. Depends on D3077 Test Plan: BK Reviewers: alangenfeld, max, prha Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3078
Summary: This is necessary for start and launch to share implementations. The start codepath will branch and call do_launch if the run launcher is configured to "hijack" the start process. This actually has a slight meaningful change (throwing UserFacingGraphQLError) so put in its own diff. Depends on D3078 Test Plan: BK Reviewers: max, alangenfeld, prha Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3079
…start with in-memory launcher Summary: Use the new synchronous in memory run launcher to start fleshing out the new codepath for doing launch only. This will allow us to flush out many of the bugs between start and launcher in a manner that is decoupled from the migration from PipelineExecutionSubprocessManager to the CliApiRunLauncher Depends on D3079 Test Plan: BK Reviewers: max, prha, alangenfeld Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3080
…uncher and use in graphql_context_test_suite Summary: This sets up the work stream to move from the usage of SubprocessPipelineExecutionManager to CliApiRunLauncher Depends on D3080 Test Plan: BK Reviewers: max, alangenfeld, prha Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3081
…t to be None Summary: This will ensure that cases where we are hijacking we do not accidentally call execute_pipeline. We can configure context variants to not have an execution_manager at all this way. Depends on D3081 Test Plan: BK Reviewers: prha, max, alangenfeld Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3082
Summary: In code where you are aware/know absolutely that you are executing in a hosted process that happens to have user code in process, it is often necessary to go back and forth between user process representations of dagster artifacts and hosted process representations. This is a bit tedious, but it should also not be supported by those abstractions natively, as it is often the wrong thing todo. Examples where this is appropriate is in testing contexts, as well as in classes like InProcessDagsterEnvironment in dagster-graphql, which is utilized when we load user code in dagit directly. To help, this adds a module dagster.utils.test.hosted_user_process to assist with these transformations. Implicitly this also adds a term "hosted user process" to describe the state where the user code is loaded directly into the host process. Depends on D3082 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3086
…d use it in test Summary: This adds an OutOfProcessDagsterEnvironment that knows how to interact with a user process through the dagster api cli. This has limited capabilities for now but enough to test simple test cases that only query metadata on full pipelines. Depends on D3086 Test Plan: BK Reviewers: max, alangenfeld Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3083
…. Found a bug Summary: This diff is a good demonstration of why both duplicated code paths are bad and why APIs like create_run are way too low level and should not exist. One should be forced to have a higher level API where you must pass an ExternalPipeline rather than force duplicated logic at every callsite. Depends on D3083 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3087
…cking codepath Summary: Continuing to move code over to the hijacking codepath. Found some weirdness in that we are using check to commmunicate graphql domain errors. Tracking here #2507 Depends on D3087 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3088
… only and move a couple tests over Summary: It turns out that this test suite has only historically been run against the sync execution manager so by just also testing against the subprocess execution manager we are finding more bugs. These two tests can be moved without change but the next one requires a fix. Depends on D3088 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3089
…ypes, test_execute_pipeline and test_execute_schedule to gql test suite Summary: Move test_compute_logs.py, test_config_types.py, test_execute_pipeline.py, test_execute_schedule.py to new test suite format. No behavior change. Only tests the single context variant that it was testing behavior. This just makes that explicit Depends on D3089 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3090
…xecution manager Summary: This moves a test that tests a disabled execution manager to the new test infrastructure. It also serves as a useful example of defining a custom graphql variant that we don't want to defined in base test file. Depends on D3090 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3091
Summary: This adds a new variant that is backed by a sqlite instance and does a synchronous hijack. We will spread this through the test suite to ferret out all differences between the start and launch codepaths that can be detected by our test suite. Depends on D3091 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3092
…line_execution_for_created_run Summary: We are going to use this function to hijack start_pipeline_execution_for_created_run codepath. Had to change the code to throw a UserFacingGraphQLError rather than just return it. Depends on D3092 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3093
…d a test. Summary: Enable in-memory hijacking for the entire test_execute_pipeline set suite. Depends on D3093 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3094
…suite Summary: With the sync cli api launcher available we can use that to do these types of tests. Depends on D3094 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3095
Summary: Move tests in test_rexecution to a proper graphql context test suite. This allows the elimination of the InMemoryRunLauncher (at this point duplicative) and sets up hijacking for the re-execution code path. Depends on D3095 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3096
Summary: Make all test_reexecution.py tests work on applicable variants, including start hijacking variants. Depends on D3096 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3097
Summary: This should be no behavior change. This just encodes the identical previous behavior using the context variants. Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3098
Summary: Adds the the first hijacking run launcher to test_retry_execution.py to test that code path. Depends on D3098 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3099
Summary: More recent pylint checks more stuff, and specifically it doesn't seem to understand `six.with_metaclass(ABCMeta)` so we need to disable those checks for now. I don't move pylint to 2.5.2 until D3109, because of pylint-dev/pylint#3648 - the long list of args to pylint seems to trigger a newly-introduced pylint bug in 2.5.x; the refactor in D3109 means we no longer trigger that bug. Test Plan: buildkite Reviewers: alangenfeld, sandyryza, max Reviewed By: sandyryza Subscribers: schrockn Differential Revision: https://dagster.phacility.com/D3114
Summary:
fix `!(tag.key in ["dagster/is_resume_retry", "dagster/step_selection"])` in D3065
it would always be false
Test Plan:
`dagster/step_selection` doesn't get passed to a child run e.g. a full pipeline run
{F146367}
Reviewers: max, bengotow, prha
Reviewed By: prha
Differential Revision: https://dagster.phacility.com/D3102
DavidKatz-il
pushed a commit
that referenced
this pull request
Jun 16, 2020
* Add dagster-azure package with various storage components This adds the following components based on Azure Data Lake Storage Gen2 (and Azure Blob Storage where appropriate): - ADLS2FileCache and adls2_file_cache - ADLS2FileManager - ADLS2IntermediateStore - ADLS2ObjectStore - the adls2_resource providing direct access to Azure Data Lake Storage - the adls2_system_storage system storage This is pretty similar to the S3 implementation, the main difference being configuration: Azure's SDK requires credentials to be passed explicitly, so the credential is expected in configuration. Tests currently require an access key to complete any tests marked 'nettest'. * Rename Fake Azure classes and modules to more English-friendly names * Add ADLS2Resource to wrap ADLS2/Blob clients * Fix import order in dagster-azure * Add dagster-azure to install_dev_python_modules make target * Include azure-storage-blob in dagster-azure requirements * Remove unused variable in tests * Don't install dagster-azure as part of install_dev_python_modules make target * Remove accidentally committed Azure Blob object/intermediate store implementations These work but have no immediate use case and no tests, so seem like an unnecessary maintenance burden. This commit can be reverted if they're needed! * Wrap potentially incompatible imports to add a custom warning This centralizes the various azure-storage/azure-core imports and wraps them, plus the snowflake-connector import, in a try/except block, adding a custom warning with a suggested solution if the import fails. * Add README to dagster-azure and note about incompatibility to dagster-snowflake's README * Isort * Set buildkite container for dagster-azure tests * Env variables in buildkite for Azure * Add dagster-azure package with various storage components Summary: This adds the following components based on Azure Data Lake Storage Gen2 (and Azure Blob Storage where appropriate): - ADLS2FileCache and adls2_file_cache - ADLS2FileManager - ADLS2IntermediateStore - ADLS2ObjectStore - the adls2_resource providing direct access to Azure Data Lake Storage - the adls2_system_storage system storage This is pretty similar to the S3 implementation, the main difference being configuration: Azure's SDK requires credentials to be passed explicitly, so the credential is expected in configuration. Tests currently require an access key to complete any tests marked 'nettest'. Rename Fake Azure classes and modules to more English-friendly names Add ADLS2Resource to wrap ADLS2/Blob clients Fix import order in dagster-azure Add dagster-azure to install_dev_python_modules make target Include azure-storage-blob in dagster-azure requirements Remove unused variable in tests Don't install dagster-azure as part of install_dev_python_modules make target Remove accidentally committed Azure Blob object/intermediate store implementations These work but have no immediate use case and no tests, so seem like an unnecessary maintenance burden. This commit can be reverted if they're needed! Wrap potentially incompatible imports to add a custom warning This centralizes the various azure-storage/azure-core imports and wraps them, plus the snowflake-connector import, in a try/except block, adding a custom warning with a suggested solution if the import fails. Add README to dagster-azure and note about incompatibility to dagster-snowflake's README Isort Set buildkite container for dagster-azure tests Merge pull request #1 from dagster-io/dagster-azure Set buildkite container for dagster-azure tests Env variables in buildkite for Azure Test Plan: bk Differential Revision: https://dagster.phacility.com/D3238 * more buildkite * tox * Run black on dagster_snowflake * Add pylint to dagster-azure tox.ini * Explicitly specify __all__ for dagster_azure utils modules * Fix black issues Co-authored-by: Sandy Ryza <[email protected]> Co-authored-by: Sandy Ryza <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.