Page MenuHomePhabricator

(Runs 2/N) Create PipelineRunData value object
ClosedPublic

Authored by schrockn on Tue, Sep 3, 9:06 PM.

Details

Reviewers
natekupp
Group Reviewers
Restricted Project
Commits
R1:34c3d8196555: (Runs 2/N) Create PipelineRunData value object
Summary

This removes the pernicious pattern of blindly passing along kwargs
into the create_run method of run storage. Instead we create a value
object, which should also be much easier to serialize.

This will represent what needs to be persisted in any particular runs backend, whereas the current PipelineRun object will be augmented with state like what subscribers are active, etc.

Test Plan

Buildkite

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

schrockn created this revision.Tue, Sep 3, 9:06 PM
schrockn updated this revision to Diff 4192.Tue, Sep 3, 10:51 PM
schrockn added a reviewer: Restricted Project.

up

schrockn edited the summary of this revision. (Show Details)Tue, Sep 3, 10:52 PM
natekupp accepted this revision.Wed, Sep 4, 3:53 AM
natekupp added a subscriber: natekupp.
natekupp added inline comments.
python_modules/dagster/dagster/core/storage/pipeline_run.py
157–159

hoorary for immutable data, this is much cleaner

python_modules/dagster/dagster/core/storage/runs.py
279

should this be a staticmethod on PipelineRun?

This revision is now accepted and ready to land.Wed, Sep 4, 3:53 AM
schrockn updated this revision to Diff 4212.Wed, Sep 4, 2:03 PM

up

python_modules/dagster/dagster/core/storage/runs.py
279

yup

schrockn updated this revision to Diff 4213.Wed, Sep 4, 2:06 PM

from_sql_row

Harbormaster completed remote builds in B3371: Diff 4213.
schrockn updated this revision to Diff 4215.Wed, Sep 4, 2:38 PM

env_config --> environment_dict

schrockn added inline comments.Wed, Sep 4, 2:50 PM
python_modules/dagster/dagster/core/storage/runs.py
279

actually nvm this is really an internal implementation detail of FilesystemRunStorage because it is dependent on the order from the sql query. will rename to make it more clear

This revision was automatically updated to reflect the committed changes.