Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exp queue: Add two command queue remove and queue kill #7721

Merged
merged 1 commit into from
May 18, 2022

Conversation

karajan1001
Copy link
Contributor

fix: #7591

  1. Add a new command dvc queue
  2. Add two sub-command dvc queue remove and dvc queue kill
  3. Add a unit test to test them

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

wait for #7714

@karajan1001 karajan1001 requested a review from a team as a code owner May 10, 2022 11:28
@karajan1001 karajan1001 requested review from skshetry and removed request for a team May 10, 2022 11:28
@karajan1001
Copy link
Contributor Author

Current UI

$ dvc queue --help
usage: dvc queue [-h] [-q | -v] {remove,kill} ...

Commands to manage dvc task queue.
Documentation: <https://man.dvc.org/queue>

positional arguments:
  {remove,kill}  Use `dvc queue CMD --help` to display command-specific help.
    remove       Remove experiments in queue
    kill         Kill experiments in queue

optional arguments:
  -h, --help     show this help message and exit
  -q, --quiet    Be quiet.
  -v, --verbose  Be verbose.

$ dvc queue remove --help
usage: dvc queue remove [-h] [-q | -v] [--all] [<experiment> [<experiment> ...]]

Remove experiments in queue
Documentation: <https://man.dvc.org/queue/remove>

positional arguments:
  <experiment>   Experiments in queue to remove.

optional arguments:
  -h, --help     show this help message and exit
  -q, --quiet    Be quiet.
  -v, --verbose  Be verbose.
  --all          Remove all experiments in queue.

$ dvc queue kill --help
usage: dvc queue kill [-h] [-q | -v] [<experiment> [<experiment> ...]]

Kill experiments in queue
Documentation: <https://man.dvc.org/queue/kill>

positional arguments:
  <experiment>   Experiments in queue to kill.

optional arguments:
  -h, --help     show this help message and exit
  -q, --quiet    Be quiet.
  -v, --verbose  Be verbose.

@karajan1001 karajan1001 added A: experiments Related to dvc exp A: task-queue Related to task queue. A: cli Related to the CLI labels May 10, 2022
@dberenbaum
Copy link
Collaborator

Should I wait for #7714 before testing this?

@karajan1001 karajan1001 force-pushed the fix7591 branch 2 times, most recently from 023001f to 5f77797 Compare May 14, 2022 10:10
Copy link
Contributor

@pmrowla pmrowla left a comment

Choose a reason for hiding this comment

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

Just a couple minor things, LGTM otherwise

dvc/commands/queue/kill.py Outdated Show resolved Hide resolved
dvc/commands/queue/remove.py Outdated Show resolved Hide resolved
dvc/commands/queue/remove.py Outdated Show resolved Hide resolved
dvc/commands/queue/__init__.py Outdated Show resolved Hide resolved
@pmrowla
Copy link
Contributor

pmrowla commented May 16, 2022

@karajan1001 there's a linting error now (unused import), just fix that and then squash your commits and this can be merged

@karajan1001
Copy link
Contributor Author

karajan1001 commented May 16, 2022

Because it relates CLI, I'd prefer to wait for Dave's suggestion.
And wondered why always there are some tests failed?

fix: iterative#7591
1. Add a new command `dvc queue`
2. Add two sub-command `dvc queue remove` and `dvc queue kill`
3. Add a unit test to test them

Co-authored-by: Peter Rowlands (λ³€κΈ°ν˜Έ) <peter@pmrowla.com>
@pmrowla
Copy link
Contributor

pmrowla commented May 16, 2022

The exp show tests are flaky because they haven't been updated to properly work with the celery changes (basically there are some race conditions where exp show doesn't wait for celery tasks to finish completely that can lead to the flaky results).

@karajan1001 karajan1001 self-assigned this May 17, 2022
@karajan1001 karajan1001 removed the request for review from skshetry May 17, 2022 09:04
@karajan1001 karajan1001 linked an issue May 17, 2022 that may be closed by this pull request
2 tasks
@karajan1001
Copy link
Contributor Author

@dberenbaum Could you please give help on the product side?

@pmrowla
Copy link
Contributor

pmrowla commented May 17, 2022

I think it may be easier for us to just merge these PRs into the dev branch so that testing can be done w/all of the commands available, and then iterate/fix bugs from there

@dberenbaum
Copy link
Collaborator

I think it may be easier for us to just merge these PRs into the dev branch so that testing can be done w/all of the commands available, and then iterate/fix bugs from there

How soon do you expect to merge? If it's in the next couple days, I will wait. Otherwise, I'll try to look at what I can now.

@karajan1001
Copy link
Contributor Author

karajan1001 commented May 18, 2022

I think it may be easier for us to just merge these PRs into the dev branch so that testing can be done w/all of the commands available, and then iterate/fix bugs from there

How soon do you expect to merge? If it's in the next couple days, I will wait. Otherwise, I'll try to look at what I can now.

I think we can wait until the next couple of days.

@pmrowla pmrowla merged commit fe08e27 into iterative:dvc-task-dev May 18, 2022
@karajan1001 karajan1001 deleted the fix7591 branch May 18, 2022 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: cli Related to the CLI A: experiments Related to dvc exp A: task-queue Related to task queue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exp queue: remove queued experiments
3 participants