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

add support for foreach target #8210

Merged
merged 1 commit into from
Sep 6, 2022
Merged

add support for foreach target #8210

merged 1 commit into from
Sep 6, 2022

Conversation

skshetry
Copy link
Member

@skshetry skshetry commented Aug 30, 2022

  • checkout1
  • commit2
  • dag
  • fetch1
  • freeze/unfreeze4
  • pull1
  • push1
  • remove3
  • status
  • exp run (already supported)
  • repro (already supported)
  • stage list (already supported)

Closes #7323.

Notes:

  1. checkout might fail if any of the hash_info from the foreach group is missing (it'll still checkout). Same with pull/push/fetch.
  2. commit might fail if the workspace has any output missing from the foreach group (this is a bit annoying).
  3. remove is not supported as it needs to modify templated dvc.yaml file, which we don't support yet. We can skip it for now. Internally in dvc, we don't have a "foreach group" to remove, it's compiled away, so we only know about individual stages that it generates. So removing a definition for that foreach group requires custom logic to handle foreach group, and then need to imitate whatever behaviour we have for remove, i.e unprotect from cache, remove from dvc.lock and dvc.yaml, remove .gitignore entries, etc.
  4. freeze/unfreeze is similar to 3. We don't modify parameterized stages and would require custom handling for that. We try not to modify dvc.yaml as much as possible.

Related: #7462, #7385

@skshetry skshetry requested review from efiop and dberenbaum August 31, 2022 04:55
@skshetry skshetry marked this pull request as ready for review August 31, 2022 04:55
@skshetry skshetry self-assigned this Aug 31, 2022
@skshetry skshetry added A: cli Related to the CLI feature is a feature labels Aug 31, 2022
@skshetry skshetry force-pushed the foreach branch 2 times, most recently from d1272ed to f2e9c43 Compare August 31, 2022 05:38
Copy link
Collaborator

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

LGTM! The edge cases you mention are fine to deprioritize.

@skshetry skshetry merged commit 071b2b8 into iterative:main Sep 6, 2022
@skshetry skshetry deleted the foreach branch September 6, 2022 00:54
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 feature is a feature
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

commit, status: cannot address all foreach stage targets
2 participants