Skip to content

external_repo: do not try too hard to fix upstream#3512

Merged
efiop merged 4 commits into
treeverse:masterfrom
skshetry:bare-repo-clone
Mar 20, 2020
Merged

external_repo: do not try too hard to fix upstream#3512
efiop merged 4 commits into
treeverse:masterfrom
skshetry:bare-repo-clone

Conversation

@skshetry
Copy link
Copy Markdown
Collaborator

@skshetry skshetry commented Mar 19, 2020

  • ❗ I have followed the Contributing to DVC checklist.

  • 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

This is a good-enough fix for #3451. At the moment, local remote does not work at all and treats it like how it'd be if it was a remote dvc repo. It's not clear if we should support this at all (ref: #2756, #3378)

Fix #3451.

@skshetry skshetry self-assigned this Mar 19, 2020
@skshetry skshetry marked this pull request as ready for review March 19, 2020 14:34
@skshetry skshetry changed the title [WIP] external_repo: do not try too hard to fix upstream external_repo: do not try too hard to fix upstream Mar 19, 2020
@skshetry skshetry requested review from Suor, efiop, gurobokum, pared and pmrowla and removed request for Suor March 19, 2020 14:35
@skshetry skshetry added the bug Did we break something? label Mar 19, 2020
Comment thread tests/func/test_import.py Outdated
Comment thread dvc/external_repo.py
Comment thread dvc/external_repo.py Outdated
Comment thread dvc/external_repo.py
Comment on lines 127 to 128
if not os.path.isdir(self.url):
return
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This check is redundant here as it should be covered by below try/except block. But, I think, this makes the intention clearer, so, keeping it for now. :)

@skshetry skshetry requested a review from efiop March 20, 2020 02:45
@efiop
Copy link
Copy Markdown
Contributor

efiop commented Mar 20, 2020

Thank you @skshetry ! 🙏

@efiop efiop merged commit 205a605 into treeverse:master Mar 20, 2020
@skshetry skshetry deleted the bare-repo-clone branch March 20, 2020 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Did we break something?

Projects

None yet

Development

Successfully merging this pull request may close these issues.

import: does not work from bare git repo

2 participants