Skip to content

Conversation

@dberenbaum
Copy link
Contributor

Continuing from #220 (comment), the issue appears to be from having the cache helper enabled in my git config:

[credential]
	helper = cache

I get the prompt as expected when I disable this. However, when it's enabled, I don't get a prompt, even if I have no credentials cached. For example, this fails:

$ git credential-cache exit
$ dvc get -v https://github.com/dberenbaum/config-files vimrc

The credential helper does not fail but returns an empty output in this scenario.

I notice we don't have any tests for any of the credential helper logic. Do we need to add something?

I don't mind if you take this over @pmrowla, but once I found the issue it wasn't any extra work to submit the PR.

@dberenbaum dberenbaum requested a review from pmrowla March 30, 2023 18:41
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Patch coverage: 94.73% and project coverage change: +1.64 🎉

Comparison is base (23b23dc) 79.60% compared to head (17c8a9b) 81.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
+ Coverage   79.60%   81.25%   +1.64%     
==========================================
  Files          26       27       +1     
  Lines        3776     3814      +38     
  Branches      663      668       +5     
==========================================
+ Hits         3006     3099      +93     
+ Misses        675      608      -67     
- Partials       95      107      +12     
Impacted Files Coverage Δ
src/scmrepo/git/credentials.py 52.19% <66.66%> (+26.06%) ⬆️
tests/test_credentials.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pmrowla pmrowla self-assigned this Mar 31, 2023
@pmrowla pmrowla merged commit b97d042 into main Mar 31, 2023
@pmrowla pmrowla deleted the cred-helper-empty branch March 31, 2023 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants