Skip to content

Conversation

@skshetry
Copy link
Collaborator

@skshetry skshetry commented Mar 27, 2023

  1. Pings Studio on exp remove
  2. Does not fail on any error during request.post
  3. UI improvement on exp remove

Closes #9227.

1) Pings Studio on exp remove
2) Does not fail on any error during request.post
3) UI improvement on exp remove
@skshetry skshetry added enhancement Enhances DVC A: experiments Related to dvc exp skip-changelog Skips changelog labels Mar 27, 2023
Comment on lines +58 to +59
# TODO: Should we use git_remote to associate with Studio project
# instead of using `git ls-remote` on fallback?
Copy link
Collaborator Author

@skshetry skshetry Mar 27, 2023

Choose a reason for hiding this comment

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

@dberenbaum, a bit of an edge case here.
What project/repository should we associate on dvc exp push origin? The origin remote url, or where the upstream remote was set to (during git push —set-upstream)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's an explicit remote, I think it should be the origin remote url. If the user wants to use a different remote, then they shouldn't specify origin, right?

WRT using the upstream remote, there's some very old discussion in #6332 (comment) and #6427. I agree it would be nice to have some way to use the upstream remote, but I think it's not that simple since we aren't pushing a branch and it's not a blocker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I meant it in terms of a Studio project. We send remote url (aka repo_url) to Studio.
At the moment, we are sending —set-upstream url instead of the remote that is passed as argument on exp-push or exp-remove.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thinking about it more, it does not make sense to pass a different repo_url if you have pushed to a separate remote. I am changing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, makes sense, thanks @skshetry!

@skshetry skshetry merged commit 1bc4c1d into treeverse:main Mar 27, 2023
@skshetry skshetry deleted the studio-exp-share-refactor branch March 27, 2023 17:06
daavoo pushed a commit that referenced this pull request Mar 28, 2023
studio experiments share refactor

1) Pings Studio on exp remove
2) Does not fail on any error during request.post
3) UI improvement on exp remove
snorkelopstesting1-a11y pushed a commit to snorkel-marlin-repos/iterative_dvc_pr_9248_2e18f3ee-0555-4d7f-a562-490eed846031 that referenced this pull request Oct 22, 2025
snorkelopsstgtesting1-spec added a commit to snorkel-marlin-repos/iterative_dvc_pr_9248_2e18f3ee-0555-4d7f-a562-490eed846031 that referenced this pull request Oct 22, 2025
…t fail on error

Merged from original PR #9248
Original: treeverse/dvc#9248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: experiments Related to dvc exp enhancement Enhances DVC skip-changelog Skips changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exp remove: ping studio to remove experiment

2 participants