repo: different cache link types info on long lasting operations #1914

pared · 2019-04-23T02:32:38Z

Have you followed the guidelines in our
Contributing document?
Does your PR affect documented changes or does it add new functionality
that should be documented? If yes, have you created a PR for
dvc.org documenting it or at
least opened an issue for it? If so, please add a link to it.

Fixes #1890

Example:
https://asciinema.org/a/2XsHkAUcUfx1aHfmUV7atnpoJ

dvc/config.py

dvc/progress.py

dvc/remote/local.py

tests/func/test_checkout.py

shcheklein · 2019-04-23T17:51:01Z

@pared @efiop slow_checkout_warning - I think we should take into account other case like add, import - etc. Probably there should be a single option and a single warning for all of them.

pared · 2019-04-24T02:12:09Z

Modified behaviour to analyze linking time, added asciinema example to this PR and issue

efiop · 2019-04-24T15:20:13Z

@pared please rebase.

dvc/remote/local/__init__.py

dvc/remote/local/link_decorator.py

efiop · 2019-04-24T21:02:55Z

dvc/stage.py

So now it will print a warning for each stage that it checks out? 🙂

dvc/repo/reproduce.py

efiop · 2019-04-25T16:59:15Z

dvc/remote/local/slow_link_detection.py

All caps is usually used for constants, so if you wish to modify it you should probably not use all caps, since it is no longer a constant :)

efiop · 2019-04-25T17:55:11Z

tests/unit/remote/test_slow_link_detection.py

why not just use with caplog.at_level(INFO) ?

efiop · 2019-04-25T17:57:12Z

dvc/remote/local/slow_link_detection.py

does it actually need to know remote_local? Or should it simply be (self, *args, **kwargs)?

it can be self, I just named it to preserve information what user can expect here

@pared not sure I follow. remote_local is not used, you are just passing it through, aren't you? If so, your __call__ doesn't care about it.

Oh, you are right, it matters only for guard, not for decorator class.

efiop · 2019-04-25T18:41:55Z

tests/unit/remote/test_slow_link_detection.py

So is this needed now? At least the naming :)

Do you even need a mock for it though? You are passing it as a function that your decorator should wrap and call, but I don't see you doing anything with it later.

I will change it so that we can make sure that arguments are passed properly

tests/unit/remote/test_slow_link_detection.py

dvc/remote/local/slow_link_detection.py

efiop · 2019-04-26T10:03:36Z

dvc/remote/local/slow_link_detection.py

Feels like the logic is backwards. Shouldn't was_displayed be False by default? And when it is displayed then it should set was_displayed to True? Or am I missing something?

efiop

Thank you! :)

pared force-pushed the 1890 branch 2 times, most recently from 8b8737d to f91856a Compare April 23, 2019 02:47

efiop approved these changes Apr 23, 2019

View reviewed changes

dvc/config.py Outdated Show resolved Hide resolved

dvc/progress.py Outdated Show resolved Hide resolved

dvc/remote/local.py Outdated Show resolved Hide resolved

tests/func/test_checkout.py Outdated Show resolved Hide resolved

pared force-pushed the 1890 branch from f91856a to 3cdde01 Compare April 24, 2019 01:59

shcheklein mentioned this pull request Apr 24, 2019

add a user guide page about the optimization (advanced mode) treeverse/dvc.org#265

Closed

6 tasks

pared requested a review from efiop April 24, 2019 02:23

pared changed the title ~~[WIP] checkout: different cache link types info on long lasting checkout~~ checkout: different cache link types info on long lasting checkout Apr 24, 2019

pared changed the title ~~checkout: different cache link types info on long lasting checkout~~ link: different cache link types info on long lasting linking Apr 24, 2019

pared changed the title ~~link: different cache link types info on long lasting linking~~ remote: local: different cache link types info on long lasting linking Apr 24, 2019

pared force-pushed the 1890 branch 3 times, most recently from 484c699 to 979d993 Compare April 24, 2019 17:22

efiop reviewed Apr 24, 2019

View reviewed changes

dvc/remote/local/__init__.py Outdated Show resolved Hide resolved

efiop suggested changes Apr 24, 2019

View reviewed changes

dvc/remote/local/link_decorator.py Outdated Show resolved Hide resolved

efiop reviewed Apr 24, 2019

View reviewed changes

dvc/stage.py Outdated

Copy link

Contributor

efiop Apr 24, 2019

Choose a reason for hiding this comment

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

So now it will print a warning for each stage that it checks out? 🙂

pared reacted with confused emoji

pared force-pushed the 1890 branch from 377f613 to f23979a Compare April 25, 2019 00:10

efiop reviewed Apr 25, 2019

View reviewed changes

dvc/repo/reproduce.py Outdated Show resolved Hide resolved

pared changed the title ~~remote: local: different cache link types info on long lasting linking~~ repo: different cache link types info on long lasting operations Apr 25, 2019

efiop reviewed Apr 25, 2019

View reviewed changes

pared force-pushed the 1890 branch 2 times, most recently from 7554469 to dab832c Compare April 25, 2019 18:28

pared requested a review from efiop April 25, 2019 18:40

efiop reviewed Apr 25, 2019

View reviewed changes

tests/unit/remote/test_slow_link_detection.py Outdated Show resolved Hide resolved

efiop reviewed Apr 25, 2019

View reviewed changes

dvc/remote/local/slow_link_detection.py Outdated Show resolved Hide resolved

pared force-pushed the 1890 branch 5 times, most recently from 90cce16 to 7502ae3 Compare April 26, 2019 02:02

pared requested a review from efiop April 26, 2019 03:51

efiop reviewed Apr 26, 2019

View reviewed changes

remote: local: different cache link types info on long lasting linking

f496ba4

pared force-pushed the 1890 branch from 7502ae3 to f496ba4 Compare April 26, 2019 17:17

pared requested a review from efiop April 26, 2019 17:22

efiop approved these changes Apr 29, 2019

View reviewed changes

efiop merged commit 435095e into treeverse:master Apr 29, 2019

pared deleted the 1890 branch November 12, 2019 08:54

repo: different cache link types info on long lasting operations #1914

repo: different cache link types info on long lasting operations #1914

Uh oh!

Conversation

pared commented Apr 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shcheklein commented Apr 23, 2019

Uh oh!

pared commented Apr 24, 2019

Uh oh!

efiop commented Apr 24, 2019

Uh oh!

Uh oh!

Uh oh!

efiop Apr 24, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

efiop Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

efiop Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiop Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

pared Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

efiop Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

pared Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiop Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiop Apr 25, 2019

Choose a reason for hiding this comment

Uh oh!

pared Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

efiop Apr 26, 2019

Choose a reason for hiding this comment

Uh oh!

efiop left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pared commented Apr 23, 2019 •

edited

Loading

efiop Apr 25, 2019 •

edited

Loading

pared Apr 25, 2019 •

edited

Loading

efiop Apr 25, 2019 •

edited

Loading

pared Apr 25, 2019 •

edited

Loading