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

live: Explicitly require exp for sending Studio updates. #507

Merged
merged 1 commit into from
Mar 27, 2023

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Mar 27, 2023

Requires using either dvc exp run or save_dvc_exp=True.

Closes #474

@daavoo daavoo force-pushed the require-exp-for-studio branch from 5de13c8 to 5cc444c Compare March 27, 2023 15:19
Copy link
Contributor Author

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

@dberenbaum The discussion in #474 was stalling but since we are moving forward in Sudio towards unifying Live Experiments and Experiment refs (i.e. https://github.com/iterative/studio/issues/5453 , https://github.com/iterative/studio/pull/5417), I think it's better to stop allowing sending updates without an experiment.

@daavoo daavoo requested a review from dberenbaum March 27, 2023 15:19
Requires using either `dvc exp run` or `save_dvc_exp=True`.

Closes #474
@daavoo daavoo force-pushed the require-exp-for-studio branch from 5cc444c to 62880aa Compare March 27, 2023 15:27
@daavoo daavoo added the A: studio Area: Studio integration label Mar 27, 2023
@dberenbaum
Copy link
Collaborator

@daavoo I'm not against this, but I still feel like I'm maybe missing the motivation. Does it help simplify something?

@daavoo
Copy link
Contributor Author

daavoo commented Mar 27, 2023

@daavoo I'm not against this, but I still feel like I'm maybe missing the motivation.

My main motivation is that I think of the use cases that this P.R forbids as lose ends.

Does it help simplify something?

It allows Studio to use baseline SHA and experiment name pair as unique identifier for all live experiments, which makes thing simpler on their end and sabes the need of changes to the client.

It also allows both running and completed experiments to be referenced by URL using the same logic

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.

It allows Studio to use baseline SHA and experiment name pair as unique identifier for all live experiments, which makes thing simpler on their end and sabes the need of changes to the client.

It also allows both running and completed experiments to be referenced by URL using the same logic

Okay, this seems like enough reason since we don't have any strong argument in the other direction. Thanks for explaining.

@daavoo daavoo merged commit 00466fa into main Mar 27, 2023
@daavoo daavoo deleted the require-exp-for-studio branch March 27, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: studio Area: Studio integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

studio: live metrics for dvc repro
2 participants