Skip to content

Conversation

pared
Copy link
Contributor

@pared pared commented May 14, 2019


@pared pared force-pushed the 1976 branch 3 times, most recently from 989dcaa to 987d6da Compare May 14, 2019 18:51
@pared pared changed the title [WIP] cache: test: allow cache to be on different drive cache: test: allow cache to be on different drive May 14, 2019
@pared pared requested a review from efiop May 14, 2019 19:06
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, still catching ValueError here seems too much, since it is only relevant for windows and ValueError could be raised in a lot of cases. Maybe it is worth checking for it separately before trying to relpath()?

Copy link
Contributor

@efiop efiop May 14, 2019

Choose a reason for hiding this comment

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

Btw, just a note: that check and relpath itself might cause quite a performance hit, as we've learned from cProfile a long time ago, so the plan was to make PathLOCAL do some caching in the future, so it doesn't have to recompute that again and again. But maybe there is a better approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Neat!

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.

Thanks! 🎉

@efiop efiop merged commit d4d4c03 into iterative:master May 16, 2019
@pared pared deleted the 1976 branch December 17, 2019 13:13
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.

Error with "dvc add" on Windows, when local cache is not on the same drive as git repo

2 participants