Skip to content

exp push/pull: add --rev, -n, -A flags#3192

Merged
jorgeorpinel merged 7 commits into
treeverse:masterfrom
karajan1001:modify_exp_push_pull
Apr 11, 2022
Merged

exp push/pull: add --rev, -n, -A flags#3192
jorgeorpinel merged 7 commits into
treeverse:masterfrom
karajan1001:modify_exp_push_pull

Conversation

@karajan1001
Copy link
Copy Markdown
Contributor

@karajan1001 karajan1001 commented Jan 18, 2022

@iesahin iesahin added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Jan 18, 2022
@karajan1001 karajan1001 force-pushed the modify_exp_push_pull branch 2 times, most recently from 2858ef3 to 2276358 Compare January 24, 2022 08:32
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-modify-exp-push-rwoid4 January 27, 2022 04:23 Inactive
@jorgeorpinel

This comment has been minimized.

Comment thread content/docs/command-reference/exp/pull.md Outdated
Comment thread content/docs/command-reference/exp/pull.md Outdated
@jorgeorpinel jorgeorpinel changed the title exp show: add --rev, -n, -A flag exp pull/push exp push/pull: add --rev, -n, -A flags Jan 27, 2022
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 January 27, 2022 11:17 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 January 27, 2022 13:16 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 January 27, 2022 13:26 Inactive
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will need an update again once #3187 (review) is resolved 🙂

Comment thread content/docs/command-reference/exp/pull.md Outdated
Comment thread content/docs/command-reference/exp/push.md Outdated
related to treeverse/dvc#7255

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 February 16, 2022 07:44 Inactive
Comment thread content/docs/command-reference/exp/pull.md Outdated
Comment thread content/docs/command-reference/exp/push.md Outdated
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 March 8, 2022 12:38 Inactive
Comment thread content/docs/command-reference/exp/push.md Outdated
Comment thread content/docs/command-reference/exp/pull.md Outdated
karajan1001 and others added 2 commits March 9, 2022 15:30
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 March 9, 2022 07:31 Inactive
@jorgeorpinel jorgeorpinel removed the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Mar 10, 2022
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 March 15, 2022 09:35 Inactive
Comment on lines +9 to +15
usage: dvc exp pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f]
[--no-cache] [-r <name>] [-j <number>] [--run-cache]
git_remote experiment

positional arguments:
git_remote Git remote name or Git URL
experiment Experiment to pull
experiment Experiments to pull
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it accepts multiple experiments, shouldn't the usage block look different?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related to #3340?
Excuse me, do you mean

usage: dvc exp pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f]
                    [--no-cache] [-r <name>] [-j <number>] [--run-cache]
                    git_remote experiments

or

usage: dvc exp pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f]
                    [--no-cache] [-r <name>] [-j <number>] [--run-cache]
                    git_remote experiment1 experiment2 experiment3 ...

If the first one, we also need some modifications on the core repo side. If the second, this usage is generated by the argparse's help message in dvc exp pull --help.

Copy link
Copy Markdown
Contributor

@dberenbaum dberenbaum Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I get for dvc exp pull --help:

usage: dvc experiments pull [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f]
                            [--no-cache] [-r <name>] [-j <number>] [--run-cache]
                            <git_remote> [<experiment> ...]

Pull an experiment from a Git remote.
Documentation: <https://man.dvc.org/exp/pull>

positional arguments:
  <git_remote>          Git remote name or Git URL.
  <experiment>          Experiments to pull.                    

Is there any reason we wouldn't use that? Are others not seeing the same?

Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Mar 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes what you see is what I meant @dberenbaum , specifically the line

<git_remote> [<experiment> ...]

But if this is part of #3340 and other refs need to be updated as well, maybe don't include the change here at all @karajan1001 . Thanks

Comment on lines +9 to +15
usage: dvc exp push [-h] [-q | -v] [-A] [--rev <commit>] [-n <num>] [-f]
[--no-cache] [-r <name>] [-j <number>] [--run-cache]
git_remote experiment

positional arguments:
git_remote Git remote name or Git URL
experiment Experiment to push
experiment Experiments to push
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same Q

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also change line 11 to git_remote [experiment [experiment ...]]?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, can we discuss that in a separate PR? Ideally in the core PR related to this. Thanks

Comment thread content/docs/command-reference/exp/pull.md Outdated
Comment thread content/docs/command-reference/exp/push.md Outdated
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝🏼 Thanks

karajan1001 and others added 2 commits April 6, 2022 14:57
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-org-modify-exp-push-rwoid4 April 6, 2022 06:57 Inactive
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging!

@jorgeorpinel jorgeorpinel merged commit 671b2c9 into treeverse:master Apr 11, 2022
@karajan1001 karajan1001 deleted the modify_exp_push_pull branch April 12, 2022 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants