Page MenuHomePhabricator

add backfill cli script
ClosedPublic

Authored by prha on Dec 6 2019, 10:28 PM.

Details

Test Plan

ran local

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

prha created this revision.Dec 6 2019, 10:28 PM
prha updated this revision to Diff 7214.Dec 6 2019, 11:01 PM

update

prha updated this revision to Diff 7225.Dec 6 2019, 11:44 PM

update

prha updated this revision to Diff 7227.Dec 7 2019, 12:08 AM

rebase

sashank added inline comments.Dec 7 2019, 12:09 AM
python_modules/dagster/dagster/cli/pipeline.py
368

What if we gave PartitionSelectors a name attribute, registered them globally, and used them here?

...  --selector LastPartitionSelector
... --selector BackfillSelector
... --selector FailedRunsSelector # possible when we thread instance to the selector
prha updated this revision to Diff 7228.Dec 7 2019, 12:26 AM

update

sashank added inline comments.Dec 7 2019, 12:28 AM
python_modules/dagster/dagster/cli/pipeline.py
433

You can use:

@pipeline_target_command

and

create_pipeline_from_cli_args(kwargs)

To take the pipeline_name as an argument and get the pipeline from the handle.

Example: https://dagster.phacility.com/source/dagster/browse/master/python_modules/dagster/dagster/cli/pipeline.py$229-242

This is awesome. The interactive part is really nice to use.

One thing I noticed was that the tags are not being added properly, but I'm not sure why, since they are being passed into PipelineRun

python_modules/dagster/dagster/cli/pipeline.py
444–446

This must be click's fault, but if I type a single character and then hit backspace, then the entire prompt disappears.

475–481

This is great

477

Maybe truncate the middle if len(partitions) > big_number?

491

I think this needs to be environment_dict_fn_for_partition (missing fn)

493

Same here (tags_fn_for_partition)

prha planned changes to this revision.Dec 7 2019, 12:55 AM

will look into the tags issue

python_modules/dagster/dagster/cli/pipeline.py
433

Ah, yeah... I am using pipeline_target_command already (see line 401)...

I had create_pipeline_from_cli_args at first, but when we needed to load the partitions off the repo, I switched to using handle_for_repo_cli_args and had to pop things off the kwargs, because handle_for_repo_cli_args throws if it finds other args.

Ultimately, I ended up building up the repo_args manually so we could use both handle_for_repo_cli_args and create_pipeline_from_cli_args, but it is a little confusing :/

477

yeah, this is still WIP, to be honest....

i really wanted to start measuring window size, so i could tabular columns like ls, but haven't gotten around to it

491

ah, bad merge... i had that fixed in some version...

prha updated this revision to Diff 7358.Dec 10 2019, 12:02 AM

support CLI-based partition tagging, handle ambiguous partition_set arguments

sashank accepted this revision.Dec 10 2019, 2:20 AM

Seems good to land! Can keep iterating on the fancy interactive experience

This revision is now accepted and ready to land.Dec 10 2019, 2:20 AM
prha updated this revision to Diff 7451.Dec 10 2019, 5:24 PM

update

This revision was automatically updated to reflect the committed changes.