-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Studio token: make it global, make it easier to setup #9265
Comments
@shcheklein I added it to #9074. Unless I'm mistaken, it's already possible to make it global via Somewhat related to #9217. If we have some command like |
Yep, it might be enough (btw, should we use |
It is |
You mean |
Yeah, let’s wait for feedback. |
@shcheklein Can you expand? I didn't get what you mean, and I'd also like to here how else you see it being used.
@skshetry I see your point and maybe it's not a blocker, but it seems harder to get a good response when we don't make it easy to setup. Another hybrid approach would be to keep this kind of experience limited to VS Code for now, but have VS Code save it to the global DVC config. |
It has to reach out to Studio, open the browser, get the code back, save it to config. So, that's why it's not as simple as, let's say as Does it answer the question, Dave?
My 2cs on this. It's easy to do on DVC and I don't see a single reason (considering the scope) of not doing that. But it would require Studio changes first - and that's where most of complexity is. VS Code itself won't help that much or improve the experience a lot. |
We don’t have to implement everything. This is just the first iteration. This is an integration, so we have to take both Studio and DVC into account. There are still other higher priority issues in Studio to iron out. I’d prefer that experience to be smooth first. Let’s release what we have, get feedback from users, and iterate. It’s not good if we keep on increasing scope. |
We should keep in mind that this it top of the funnel, @skshetry - means that more users potentially can try the whole combination of DVC + Studio + VS Code. It's important to optimize. I would still go with this from the Studio side- if we have there capacity or not. |
@shcheklein, do you mind creating an issue on the Studio side, please? |
It exists already https://github.com/iterative/studio/issues/5158 |
Thanks. I am dropping priority to
p2-medium
|
That's fair @skshetry, it doesn't make sense until we have Studio support. I see you quickly have been reprioritizing as needed, so are you open to it being a quick follow-up once that's implemented? I don't think we need to spend more time discussing if it's only about a few days or at most a week or two difference in time. |
It depends on what auth flow will be used. It probably also depends on if we are going to support custom Studio servers or not, etc. |
Some rough idea of how the CLI experience could look (copied from #9074 (comment)): $ dvc studio login
Redirecting you to studio.iterative.ai...
Token saved to `~/.config/dvc/config`
Would you like to enable automatic experiment sharing? (Y/n) I'm not sure I like enabling auto sharing by default, but I think it should at least be an option. |
👍 - I recommend we add a link to docs site to some page explaining exactly WHAT "automatic experiment sharing" means and the implication of turning this on, and if there aren't too harsh downsides, it might be ok to even leave this on as default. but if we're asking so explicitly it doesn't matter too much either way imo |
https://github.com/iterative/studio/issues/7652 is in progress. Once that is complete, dvc can add this. Thoughts on the UI? Should we call it |
Looks like this is ready per https://github.com/iterative/studio/issues/5158 |
Will try to implement this and ask for review. |
Since token is being also used by VS Code we need to come up with a single way to manage it. I think it should be done in a way similar to
gh
app and not via DVC config per repo.We have now a bit broken experience since we have it in two places and managed differently.
cc @mattseddon
The text was updated successfully, but these errors were encountered: