Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get, get-url: add --jobs option #2122
get, get-url: add --jobs option #2122
Changes from 5 commits
20ab141
74e71aa
619e36a
f7f6e03
1bea528
7a0514a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this apply to
get
(andimport
)? I.e. does it respect the .dvc/config settings of the source repo?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it should, though might be mistaken on this cc: @efiop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's confusing. Maybe:
Or even make that last sentence an independent block quote (at the bullet indentation level).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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 ajobs
in its config - is it a problem?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood #2122 (comment) correctly, the configuration of the source repo is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For
import-url
, it goes with--jobs => remote.jobs => core.jobs => BaseTree.JOBS
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that not
BaseTree.JOBS
but more likeTree.JOBS
, as defaults are different for different clouds. For ssh it is 4 for the rest it is 4 * NCPU.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make sense to mention this here as we do in the other parts of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shcheklein It is already mentioned here. Unless I'm misunderstanding your question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Sure, could mention here.