get, get-url: add --jobs option #2122

isidentical · 2021-01-29T08:18:50Z

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc 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 causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

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

iterative/dvc#5362

per iterative/dvc#5267 (comment)

content/docs/command-reference/get.md

jorgeorpinel · 2021-02-02T15:58:13Z

content/docs/command-reference/get.md

+  default is `4`. Note that the default value can be set using the `jobs` config
+  option with `dvc remote modify`. Using more jobs may speed up the operation.


Note that the default value can be set using the jobs config option with dvc remote modify.

Does this apply to get (and import)? I.e. does it respect the .dvc/config settings of the source repo?

I.e. does it respect the .dvc/config settings of the source repo?

I believe it should, though might be mistaken on this cc: @efiop

In the source - yes, in the host - no. Meaning that if you run dvc get inside of some repo, its configs won't be used. This is consistent with all of the related commands (list/import/get). There are a few separate tasks for that, we are almost ready to get back to them, as most of the necessary pre-requisites have already been merged.

@efiop what will be the behavior after those tasks are addressed? I'm trying to understand if we need some additional clarification here (what config is being used).

@efiop ping ^^ I'm not sure I understood the comment about There are a few separate tasks for that, we are almost ready to get back to them, as most of the necessary pre-requisites have already been merged., could you clarify it a bit please?

what will be the behavior after those tasks are addressed?

Don't know right now. Maybe we'll try to use host configs too or maybe we'll require explicit --config flag or config flags.

I think the confusion here is that dvc remote modify mentioned is talking about source repo, not the host repo.

Yep, that's confusing. Maybe:

Suggested change

default is `4`. Note that the default value can be set using the `jobs` config

option with `dvc remote modify`. Using more jobs may speed up the operation.

default is `4`. Using more jobs may speed up the operation. Note that the

default value can be set in the source repo using the `jobs` config option of

`dvc remote modify`.

Or even make that last sentence an independent block quote (at the bullet indentation level).

jorgeorpinel

Just 1 Q ☝️

content/docs/command-reference/import-url.md

content/docs/command-reference/status.md

content/docs/command-reference/push.md

content/docs/command-reference/get-url.md

shcheklein · 2021-02-02T20:20:17Z

content/docs/command-reference/import-url.md

@@ -132,6 +132,10 @@ $ dvc run -n download_data \
  already exist locally and you want to "DVCfy" this state of the project (see
  also `dvc commit`).

+- `-j <number>`, `--jobs <number>` - parallelism level for DVC to download data


@isidentical just to make sure - it's not affected by any configs at all, right? (I understand that this is not about remotes in it's default mode, but we also have core.jobs or something if I remember it right)

also, what happens if we do:

dvc import-url remote://name syntax and remote specifies a jobs in its config - is it a problem?

If I understood #2122 (comment) correctly, the configuration of the source repo is used.

For import-url, it goes with --jobs => remote.jobs => core.jobs => BaseTree.JOBS

Also note that not BaseTree.JOBS but more like Tree.JOBS, as defaults are different for different clouds. For ssh it is 4 for the rest it is 4 * NCPU.

does it make sense to mention this here as we do in the other parts of this PR?

@shcheklein It is already mentioned here. Unless I'm misunderstanding your question.

Hmm, I don't see it. (I don't mean defaults, but rather that it's affected or can be affected by config. We have something like this in the other places)

I see. Sure, could mention here.

per #2122 (review)

jorgeorpinel · 2021-02-07T02:55:59Z

content/docs/command-reference/get-url.md

+  default is `4`. Note that the default value can be set using the `jobs` config
+  option with `dvc remote modify`. Using more jobs may speed up the operation.


the default value can be set using the jobs config option with dvc remote modify.

This doesn't apply to get-url right? 🙂

jorgeorpinel · 2021-02-07T02:57:35Z

content/docs/command-reference/import-url.md

+  default is `4`. Note that the default value can be set using the `jobs` config
+  option with `dvc remote modify`. Using more jobs may speed up the operation.


Same here (there are no remotes involved in the source for import-url, except with --to-remote)

per #2122 (review) and #2122 (review)

jorgeorpinel · 2021-02-07T03:04:33Z

I addressed my last commends in #2158

* cmd: --jobs option desc in sync refs per #2122 (review) * cmd: update get/import-url --jobs desc. per #2122 (review) and #2122 (review)

get, get-url: add --jobs option

20ab141

shcheklein temporarily deployed to dvc-landing-get-jobs-jz9mt16th January 29, 2021 08:19 Inactive

jorgeorpinel added 2 commits February 2, 2021 03:39

cmd: consistent --jobs desc.

74e71aa

cmd: add import-url --jobs

619e36a

per iterative/dvc#5267 (comment)

jorgeorpinel reviewed Feb 2, 2021

View reviewed changes

content/docs/command-reference/get.md Outdated Show resolved Hide resolved

jorgeorpinel temporarily deployed to dvc-landing-get-jobs-jz9mt16th February 2, 2021 15:47 Inactive

jorgeorpinel force-pushed the get-jobs branch from 076a079 to 20ab141 Compare February 2, 2021 15:47

jorgeorpinel temporarily deployed to dvc-landing-get-jobs-jz9mt16th February 2, 2021 15:57 Inactive

jorgeorpinel reviewed Feb 2, 2021

View reviewed changes

jorgeorpinel approved these changes Feb 2, 2021

View reviewed changes

jorgeorpinel temporarily deployed to dvc-landing-get-jobs-jz9mt16th February 2, 2021 16:40 Inactive

jorgeorpinel reviewed Feb 2, 2021

View reviewed changes

content/docs/command-reference/import-url.md Show resolved Hide resolved

jorgeorpinel added the 2.0 release label Feb 2, 2021

This comment has been minimized.

Sign in to view

shcheklein reviewed Feb 2, 2021

View reviewed changes

content/docs/command-reference/status.md Outdated Show resolved Hide resolved

shcheklein reviewed Feb 2, 2021

View reviewed changes

content/docs/command-reference/push.md Outdated Show resolved Hide resolved

shcheklein reviewed Feb 2, 2021

View reviewed changes

content/docs/command-reference/get-url.md Show resolved Hide resolved

shcheklein reviewed Feb 2, 2021

View reviewed changes

jorgeorpinel self-assigned this Feb 3, 2021

jorgeorpinel added 2 commits February 3, 2021 03:48

Merge branch 'master' into get-jobs

f7f6e03

cmd: roll back sync command changes

1bea528

per #2122 (review)

jorgeorpinel added a commit that referenced this pull request Feb 3, 2021

cmd: --jobs option desc in sync refs

46cd475

per #2122 (review)

jorgeorpinel temporarily deployed to dvc-landing-get-jobs-jz9mt16th February 3, 2021 19:11 Inactive

This was referenced Feb 3, 2021

cmd: --jobs option desc in sync refs #2145

Merged

guide: Experiment Management #2146

Merged

shcheklein pushed a commit that referenced this pull request Feb 4, 2021

cmd: --jobs option desc in sync refs (#2145)

03a736a

per #2122 (review)

shcheklein assigned isidentical and unassigned jorgeorpinel Feb 4, 2021

-j config

7a0514a

shcheklein temporarily deployed to dvc-landing-get-jobs-jz9mt16th February 5, 2021 10:48 Inactive

shcheklein approved these changes Feb 5, 2021

View reviewed changes

shcheklein merged commit a1b50de into master Feb 5, 2021

jorgeorpinel reviewed Feb 7, 2021

View reviewed changes

jorgeorpinel added a commit that referenced this pull request Feb 7, 2021

cmd: update get/import-url --jobs desc.

47ebf97

per #2122 (review) and #2122 (review)

jorgeorpinel mentioned this pull request Feb 7, 2021

cmd: update get/import-url --jobs desc. #2158

Merged

jorgeorpinel deleted the get-jobs branch February 7, 2021 03:05

shcheklein pushed a commit that referenced this pull request Feb 7, 2021

cmd: update get/import-url --jobs desc. (#2158)

198fdeb

* cmd: --jobs option desc in sync refs per #2122 (review) * cmd: update get/import-url --jobs desc. per #2122 (review) and #2122 (review)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

get, get-url: add --jobs option #2122

get, get-url: add --jobs option #2122

isidentical commented Jan 29, 2021

jorgeorpinel Feb 2, 2021 •

edited

Loading

isidentical Feb 2, 2021

efiop Feb 2, 2021

shcheklein Feb 2, 2021

shcheklein Feb 3, 2021

efiop Feb 3, 2021

jorgeorpinel Feb 4, 2021

jorgeorpinel left a comment •

edited

Loading

This comment has been minimized.

shcheklein Feb 2, 2021

jorgeorpinel Feb 3, 2021

isidentical Feb 3, 2021

This comment was marked as resolved.

This comment was marked as resolved.

efiop Feb 3, 2021

shcheklein Feb 3, 2021

efiop Feb 3, 2021 •

edited

Loading

shcheklein Feb 4, 2021

efiop Feb 4, 2021

jorgeorpinel Feb 7, 2021

jorgeorpinel Feb 7, 2021

jorgeorpinel commented Feb 7, 2021

		default is `4`. Note that the default value can be set using the `jobs` config
		option with `dvc remote modify`. Using more jobs may speed up the operation.

get, get-url: add --jobs option #2122

get, get-url: add --jobs option #2122

Conversation

isidentical commented Jan 29, 2021

jorgeorpinel Feb 2, 2021 • edited Loading

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

jorgeorpinel left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efiop Feb 3, 2021 • edited Loading

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

jorgeorpinel commented Feb 7, 2021

jorgeorpinel Feb 2, 2021 •

edited

Loading

jorgeorpinel left a comment •

edited

Loading

efiop Feb 3, 2021 •

edited

Loading