docs: locking-related updates #868

efiop · 2019-12-16T11:36:29Z

Related to #860

Disregard the recommendations below if you use Edit on GitHub button to improve the docs in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This enables GitHub to link the PR to the corresponding bug and close it automatically when PR is merged.

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

static/docs/command-reference/repro.md

shcheklein · 2019-12-16T17:11:01Z

@efiop still WIP?

static/docs/command-reference/run.md

efiop · 2019-12-16T18:52:06Z

@efiop still WIP?

Yes, I am a bit ashamed of this PR, so made it WIP to take another look 🙂Will give it another look till tomorrow and will address your concerns.

jorgeorpinel

In addition to previous comments, it would be great if we can link existing instanced of "always locked" phrase in the few docs where these exist. Thanks

…oject (instead of downloading) per iterative#859 (review), + remove "s around 'locked` in some docs (related to iterative#860 and iterative#868).

efiop · 2019-12-22T20:19:27Z

In addition to previous comments, it would be great if we can link existing instanced of "always locked" phrase in the few docs where these exist. Thanks

@jorgeorpinel Not sure what you mean. Could you clarify?

efiop · 2019-12-22T20:20:32Z

Updated this PR a bit. Don't want to make it turn into a long one, so I suppose it is better to stay with the current amount of info to start with. Open to suggestions to improve it and make it mergeable.

static/docs/command-reference/run.md

jorgeorpinel · 2019-12-23T01:02:47Z

Open to suggestions to improve it and make it mergeable.

My main general concern here at this point is that basic locking is not explained anywhere, I think. So talking about how locking still allows parallel execution seems like skipping a step 🙂

Please see #868 (review)

shcheklein · 2019-12-30T06:43:37Z

@jorgeorpinel I think it's a good first step for the "document multiple parallel repros/runs" - that's what this PR is essentially about, not "document locking in dvc run/repro" in general the @efiop named it initially. I think we do describe project lock only in the DVC files and directories? It's not enough, true. But it should be probably a separate PR to write a proper DVC internals section aboutt .dvc/lock and related stuff.

@efiop Btw, let's may be change Parallel execution to even more explicit - "Running multiple runs/repros in parallel"? Parallel execution might be an overloaded term and can lead to some confusion. If we keep it it's better to explicitly specify that we don't support in-repro parallelism, for example.

static/docs/command-reference/repro.md

shcheklein

Think about making the section name more explicit.
Some clarification in the dvc repro doc can be beneficial.

efiop · 2019-12-30T09:05:32Z

Sorry guys, will get back to this later(hopefully tomorrow). It is in my saved tickets.

jorgeorpinel

I agree with Ivan's review so approving on my end.

I think we should prioritize #876 as well though – cc @shcheklein

efiop · 2020-01-05T00:44:07Z

Sorry for the delay. I've updated the docs to separate "running other commands parallel" from "parallel stage execution". Should be more clear now.

public/static/docs/command-reference/repro.md

shcheklein · 2020-01-05T03:56:56Z

public/static/docs/command-reference/run.md

@@ -52,6 +52,14 @@ captures data and <abbr>caches</abbr> relevant <abbr>data artifacts</abbr> along
 the way. See [this example](/doc/get-started/example-pipeline) to learn more and
 try creating a pipeline.

+### Running other dvc commands in parallel


I'm not sure about this ... why do we mention this here and in repro and not in dvc add?

We also def missing any reasonable context about the project lock.

It feels that it can be a general user guide section that describes locking and when and how do we release it. How to find those files, etc.

@shcheklein because dvc add doesn't release repo lock, but repro/run do release it. So for add the section would be running dvc run/repro in parallel which is odd to say the least. Eventually we will release repo lock on checksum computations/copies too, so we will alter the docs accordingly.

Ok, I guess this behaviour is simply an expected one, hence why so many people were running into it. So it is not worth documenting it here. Will move to user-guide.

Ok, removed that note entirely. Will wait until we do similar things on hash computation/copying/downloading and will then create a locking article, that would be too incomplete to bother with right now.

shcheklein · 2020-01-05T03:59:03Z

public/static/docs/command-reference/repro.md

+See
+[Running other dvc commands in parallel](/doc/command-reference/run#running-other-dvc-commands-in-parallel).
+
+### Parallel stage execution


this is a good one! I'll @jorgeorpinel review specific language nuances, but I hope it will be useful for people. (When we at last have a How to section it can be probably moved there with a link to it from this document - just to keep a sane size)

How to section?

"How to" (name can change) - we have a bunch of tickets already that start like "write a How to **** "

I see. OK, I created #899 to determine this.

shcheklein

parallel stage execution is great!

locking looks strange, w/o context - I doubt users would get it - check my comment with some suggestions on how to move forward with it.

efiop · 2020-01-06T02:44:19Z

Removed locking explanation entirely for now, will wait until we implement the same thing for hash/copying/downloading and will then create an article about it in the user-guide. If I try to do it now it will be too incomplete.

Now this PR introduces a note about parallel execution to repro cmd reference as well as updates user-guide internal files list doc.

jorgeorpinel

Rephrasing of text in Parallel stage execution:

public/static/docs/command-reference/repro.md

public/static/docs/user-guide/dvc-files-and-directories.md

jorgeorpinel · 2020-01-06T03:29:22Z

locking looks strange, w/o context - I doubt users would get it

I had this same impression before (see #868 (review)). Do you want to assign a priority to #876 so we address it sooner than later @shcheklein?

will wait until we implement the same thing for hash/copying/downloading and will then create an article about it in the user-guide

@efiop you can probably just reuse #876 for this. Feel free to update that issue's description or add a comment.

public/static/docs/command-reference/repro.md

jorgeorpinel · 2020-01-06T05:03:15Z

Just one last comment from me, I think ^
Thanks Ruslan!

public/static/docs/command-reference/repro.md

shcheklein · 2020-01-06T20:22:31Z

@jorgeorpinel please, feel free to merge whenever you feel it's ready.

@efiop thanks!!

Related to iterative#860

efiop changed the title ~~document locking in dvc run/repro~~ [WIP] document locking in dvc run/repro Dec 16, 2019

efiop changed the title ~~[WIP] document locking in dvc run/repro~~ document locking in dvc run/repro Dec 16, 2019

efiop changed the title ~~document locking in dvc run/repro~~ [WIP] document locking in dvc run/repro Dec 16, 2019

shcheklein reviewed Dec 16, 2019

View reviewed changes

static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

shcheklein reviewed Dec 16, 2019

View reviewed changes

static/docs/command-reference/run.md Outdated Show resolved Hide resolved

jorgeorpinel suggested changes Dec 18, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

weekly-digest bot mentioned this pull request Dec 22, 2019

Weekly Digest (15 December, 2019 - 22 December, 2019) #878

Closed

efiop force-pushed the 860 branch 2 times, most recently from 28dc9a3 to 5d007cc Compare December 22, 2019 20:16

efiop changed the title ~~[WIP] document locking in dvc run/repro~~ document locking in dvc run/repro Dec 22, 2019

shcheklein reviewed Dec 22, 2019

View reviewed changes

static/docs/command-reference/run.md Outdated Show resolved Hide resolved

jorgeorpinel reviewed Dec 23, 2019

View reviewed changes

static/docs/command-reference/run.md Outdated Show resolved Hide resolved

This was referenced Dec 23, 2019

locking: explain the different mechanisms, disambiguate #880

Closed

user-guide: explain locking mechanisms that we use #876

Closed

weekly-digest bot mentioned this pull request Dec 29, 2019

Weekly Digest (22 December, 2019 - 29 December, 2019) #887

Closed

shcheklein reviewed Dec 30, 2019

View reviewed changes

static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

shcheklein requested changes Dec 30, 2019

View reviewed changes

jorgeorpinel approved these changes Jan 2, 2020

View reviewed changes

efiop force-pushed the 860 branch 2 times, most recently from 3861fbb to 1b1c1b2 Compare January 4, 2020 20:19

efiop changed the title ~~[WIP] document locking in dvc run/repro~~ document locking in dvc run/repro Jan 5, 2020

efiop requested review from shcheklein and jorgeorpinel January 5, 2020 00:44

shcheklein reviewed Jan 5, 2020

View reviewed changes

public/static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

shcheklein reviewed Jan 5, 2020

View reviewed changes

shcheklein requested changes Jan 5, 2020

View reviewed changes

efiop force-pushed the 860 branch from c737e48 to 5c353c6 Compare January 6, 2020 02:39

efiop changed the title ~~document locking in dvc run/repro~~ docs: locking-related updates Jan 6, 2020

jorgeorpinel suggested changes Jan 6, 2020

View reviewed changes

public/static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

public/static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

jorgeorpinel suggested changes Jan 6, 2020

View reviewed changes

public/static/docs/user-guide/dvc-files-and-directories.md Outdated Show resolved Hide resolved

public/static/docs/user-guide/dvc-files-and-directories.md Outdated Show resolved Hide resolved

efiop force-pushed the 860 branch from 5c353c6 to 5afd157 Compare January 6, 2020 03:59

jorgeorpinel mentioned this pull request Jan 6, 2020

how: new (sub)section (How To's) #899

Closed

5 tasks

jorgeorpinel reviewed Jan 6, 2020

View reviewed changes

public/static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

efiop force-pushed the 860 branch from 5afd157 to 80bfcd2 Compare January 6, 2020 10:00

efiop requested review from jorgeorpinel and shcheklein January 6, 2020 10:35

jorgeorpinel approved these changes Jan 6, 2020

View reviewed changes

jorgeorpinel reviewed Jan 6, 2020

View reviewed changes

public/static/docs/command-reference/repro.md Outdated Show resolved Hide resolved

shcheklein approved these changes Jan 6, 2020

View reviewed changes

docs: locking-related updates

0d8f749

Related to iterative#860

efiop force-pushed the 860 branch from 80bfcd2 to 0d8f749 Compare January 6, 2020 20:45

jorgeorpinel merged commit dd6723c into iterative:master Jan 6, 2020

efiop deleted the 860 branch January 6, 2020 23:13

weekly-digest bot mentioned this pull request Jan 12, 2020

Weekly Digest (5 January, 2020 - 12 January, 2020) #916

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: locking-related updates #868

docs: locking-related updates #868

efiop commented Dec 16, 2019 •

edited

Loading

shcheklein commented Dec 16, 2019

efiop commented Dec 16, 2019 •

edited

Loading

jorgeorpinel left a comment

This comment has been minimized.

This comment has been minimized.

efiop commented Dec 22, 2019

efiop commented Dec 22, 2019

jorgeorpinel commented Dec 23, 2019

shcheklein commented Dec 30, 2019 •

edited by efiop

Loading

shcheklein left a comment

efiop commented Dec 30, 2019

jorgeorpinel left a comment

efiop commented Jan 5, 2020

shcheklein Jan 5, 2020

efiop Jan 6, 2020

efiop Jan 6, 2020

efiop Jan 6, 2020

shcheklein Jan 5, 2020

jorgeorpinel Jan 6, 2020

shcheklein Jan 6, 2020

jorgeorpinel Jan 6, 2020

shcheklein left a comment

efiop commented Jan 6, 2020 •

edited by jorgeorpinel

Loading

jorgeorpinel left a comment •

edited

Loading

jorgeorpinel commented Jan 6, 2020

jorgeorpinel commented Jan 6, 2020

shcheklein commented Jan 6, 2020

docs: locking-related updates #868

docs: locking-related updates #868

Conversation

efiop commented Dec 16, 2019 • edited Loading

shcheklein commented Dec 16, 2019

efiop commented Dec 16, 2019 • edited Loading

jorgeorpinel left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

efiop commented Dec 22, 2019

efiop commented Dec 22, 2019

jorgeorpinel commented Dec 23, 2019

shcheklein commented Dec 30, 2019 • edited by efiop Loading

shcheklein left a comment

Choose a reason for hiding this comment

efiop commented Dec 30, 2019

jorgeorpinel left a comment

Choose a reason for hiding this comment

efiop commented Jan 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shcheklein left a comment

Choose a reason for hiding this comment

efiop commented Jan 6, 2020 • edited by jorgeorpinel Loading

jorgeorpinel left a comment • edited Loading

Choose a reason for hiding this comment

jorgeorpinel commented Jan 6, 2020

jorgeorpinel commented Jan 6, 2020

shcheklein commented Jan 6, 2020

efiop commented Dec 16, 2019 •

edited

Loading

efiop commented Dec 16, 2019 •

edited

Loading

shcheklein commented Dec 30, 2019 •

edited by efiop

Loading

efiop commented Jan 6, 2020 •

edited by jorgeorpinel

Loading

jorgeorpinel left a comment •

edited

Loading