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 push/pull/list: support git credential storage #6586

Closed
dberenbaum opened this issue Sep 9, 2021 · 3 comments
Closed

exp push/pull/list: support git credential storage #6586

dberenbaum opened this issue Sep 9, 2021 · 3 comments
Assignees
Labels
A: experiments Related to dvc exp git Related to git and git backends p2-medium Medium priority, should be done, but less important

Comments

@dberenbaum
Copy link
Contributor

Extracted from #6493.

dvc exp push/pull/list do not work today if credentials are required to be provided, such as dvc exp push to a public Github HTTP URL or dvc exp push/pull/list for a private Github HTTP URL.

Also, even if using git credential stores, in which case users don't need to manually enter credentials when performing git operations, DVC doesn't currently read these credentials and still complains that the URL is unauthenticated. Note that dulwich seems to have support for this: https://github.com/dulwich/dulwich/blob/cdc93e058fb28e4884f8d6a23b4d79b14d2a8098/dulwich/client.py#L2222.

As discussed in the linked PR, it would be helpful to read from the credential store and ask for the username and password if the HTTPClientUnauthorized exception is thrown. It would also be helpful to memoize to avoid having to repeatedly ask for credentials in the case of auto-pushing checkpoints. This might be generally useful because obtaining a new git client for each checkpoint seems wasteful regardless.

@pmrowla
Copy link
Contributor

pmrowla commented May 4, 2022

this feature req applies to DVC in general (and not just exp sharing) now that clone has been completely migrated to using dulwich/pygit instead of gitpython

related: #6586 #7674

@pmrowla
Copy link
Contributor

pmrowla commented May 4, 2022

Adding this to the backlog as follow-up to the workaround in #7674

from #7674 (comment):

Ideally we would implement support for credential helpers in either dulwich (jelmer/dulwich#873) or libgit2/pygit (libgit2/libgit2#4873)

Adding support for this requires implementing the git credentials storage protocol in dulwich and/or libgit. On the dulwich side, essentially this just means:

  • reading the name/path of an executable from the git config
  • run the executable in subprocess with the appropriate action args (get/store/erase)
  • read/write the key=value protocol values to/from the process as described in the git docs

We also probably only need read-only support for now (i.e. likely only need to implement the get action)

@pmrowla pmrowla added p2-medium Medium priority, should be done, but less important git Related to git and git backends type: enhancement labels May 4, 2022
@pmrowla pmrowla changed the title exp push/pull/list: support credentials exp push/pull/list: support git credential storage May 5, 2022
@dtrifiro
Copy link
Contributor

Released with 2.25.0

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 git Related to git and git backends p2-medium Medium priority, should be done, but less important
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants