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

investigate error handlling 'dvc metrics show -a' fetch remote branch #2213

Closed
AlessandroBregoli opened this issue Jul 1, 2019 · 6 comments · Fixed by #2241
Closed

investigate error handlling 'dvc metrics show -a' fetch remote branch #2213

AlessandroBregoli opened this issue Jul 1, 2019 · 6 comments · Fixed by #2241
Assignees
Labels
bug Did we break something? p0-critical Critical issue. Needs to be fixed ASAP.

Comments

@AlessandroBregoli
Copy link

Executing 'dvc metrics show -a` ask for github credentials in loop. If there is an ssh key for github the command work but is really slow (more than a minute). If no remote is set for git all work without problem.

Please provide information about your setup
DVC version 0.50.1, Installed with pip3 on ubuntu 18.04

@efiop efiop added bug Did we break something? p0-critical Critical issue. Needs to be fixed ASAP. labels Jul 1, 2019
@efiop
Copy link
Contributor

efiop commented Jul 1, 2019

@efiop
Copy link
Contributor

efiop commented Jul 2, 2019

@efiop
Copy link
Contributor

efiop commented Jul 2, 2019

This is where we actually need that branch fetching https://github.com/iterative/dvc/blob/0.50.1/dvc/external_repo.py#L66

@efiop
Copy link
Contributor

efiop commented Jul 2, 2019

Related #2201

@efiop efiop changed the title error handlling 'dvc metrics show -a' fetch remote branch investigate error handlling 'dvc metrics show -a' fetch remote branch Jul 2, 2019
@efiop efiop assigned pared and Suor and unassigned pared Jul 2, 2019
@Suor
Copy link
Contributor

Suor commented Jul 5, 2019

Diagnose: show-ref --verify requires exact ref path like refs/heads/some-branch and we use non-qualified branch names got from repo.scm.list_branches().

I suggest to never fetch anything as a side effect. We should always work with working tree and git repo state in its actual state. @efiop do we still need that ._try_fetch_from_remote()?

@efiop
Copy link
Contributor

efiop commented Jul 5, 2019

@Suor Indeed, after #2218 we don't need _try_fetch_from_remote() at all. Thank you for investigating!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Did we break something? p0-critical Critical issue. Needs to be fixed ASAP.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants