Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Sep 15, 2019

Fix #2423

TODO:

  • Write a test for DownloadError
  • Wrap dependency.repo.fetch to always return DownloadError (what about a git clone error? what about a find_out_by_relpath error? This will continue halting the repo.fetch operation)

@efiop efiop changed the title [WIP] pull works with imported stage [WIP] make dvc pull work with imported stages Sep 19, 2019
@ghost ghost changed the title [WIP] make dvc pull work with imported stages pull: download data from imported stages Sep 19, 2019
@ghost
Copy link
Author

ghost commented Sep 20, 2019

@efiop , would you mind taking another look?

@ghost ghost requested a review from efiop September 20, 2019 01:25
Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

For the record: discussed during our meeting.

@ghost ghost changed the title pull: download data from imported stages [wip] pull: download data from imported stages Sep 21, 2019
@ghost ghost mentioned this pull request Sep 21, 2019
@ghost ghost requested review from Suor and efiop September 21, 2019 18:09
@ghost ghost requested a review from efiop September 21, 2019 19:50
@ghost ghost changed the title [wip] pull: download data from imported stages pull: download data from imported stages Sep 21, 2019
Co-Authored-By: Ruslan Kuprieiev <kupruser@gmail.com>
@ghost ghost requested a review from efiop September 21, 2019 21:14
@efiop
Copy link
Contributor

efiop commented Sep 21, 2019

Alright, let's go with this as a temporary solution. We will revisit this with a refactor soon.

@efiop efiop merged commit ae90904 into treeverse:master Sep 21, 2019
@ghost ghost deleted the fix-2345 branch September 21, 2019 22:28
@Suor
Copy link
Contributor

Suor commented Oct 5, 2019

My review point

This still looks wrong. If remote is not configured and there are imports than the error will be silently ignored.

was lost along the way. I've created a new issue for it - #2579

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.

fetch/pull: download cache for imported data from external repos

2 participants