Skip to content
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

ref: improve add/import* to-cache/remote info and examples #2302

Merged
merged 32 commits into from
Mar 19, 2021
Merged

Conversation

jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel commented Mar 15, 2021

@shcheklein shcheklein temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 03:26 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 03:30 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 04:19 Inactive
@jorgeorpinel jorgeorpinel marked this pull request as ready for review March 15, 2021 05:33
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 05:33 Inactive
Copy link
Contributor Author

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Just one Q on a fix to the import ref. Cc @efiop in case you're around (I'll double check this myself tmr if not so no worries). Thanks

Comment on lines 41 to 43
(ℹ️) DVC won't push or pull imported data to/from
[remote storage](/doc/command-reference/remote), it will rely on it's original
source.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was wrong right? import-url states the opposite.

Copy link
Contributor

Choose a reason for hiding this comment

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

import and import-url have different behavior, the original docs for each command were correct

Copy link
Contributor Author

@jorgeorpinel jorgeorpinel Mar 15, 2021

Choose a reason for hiding this comment

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

Interestinggggg 🤔 OK will roll back, thanks! ✔️

content/docs/command-reference/import.md Outdated Show resolved Hide resolved
Comment on lines 369 to 371
One option is to setup an
[external cache](/doc/use-cases/shared-development-server#configure-the-external-shared-cache)
in a location that can handle the data. Another is to use the `--to-remote`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we need a full to-cache example in import-url (like in https://dvc.org/doc/command-reference/add#example-transfer-to-the-cache).

Copy link
Contributor

@isidentical isidentical Mar 15, 2021

Choose a reason for hiding this comment

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

import-url doesn't have the to-cache ability, but rather work on its own (like you just import an URL with it and it puts it to your cache, but not in a chunked way).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point. But then maybe we should explain about the chunking (transferring) a bit more... I'll review again ⌛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But with --to-cache the chinked transfer does happen, right @isidentical ? Just double checking. Already updated in c393212, PTAL.

@shcheklein
Copy link
Member

@jorgeorpinel please split this into 3-4 PRs. It has too many things at once. Makes it hard to review.

content/docs/command-reference/add.md Outdated Show resolved Hide resolved
Comment on lines 369 to 371
One option is to setup an
[external cache](/doc/use-cases/shared-development-server#configure-the-external-shared-cache)
in a location that can handle the data. Another is to use the `--to-remote`
Copy link
Contributor

@isidentical isidentical Mar 15, 2021

Choose a reason for hiding this comment

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

import-url doesn't have the to-cache ability, but rather work on its own (like you just import an URL with it and it puts it to your cache, but not in a chunked way).

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 07:13 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 07:20 Inactive
@jorgeorpinel jorgeorpinel changed the title Misc. updates (new) ref: improve add/import to-cache/remote info and examples Mar 15, 2021
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Mar 15, 2021

OK @shcheklein sorry about that. I removed the changes to config and plots from here (stashed for now). Everything else is the same change (updated PR title too).

@jorgeorpinel jorgeorpinel changed the title ref: improve add/import to-cache/remote info and examples ref: improve add/import* to-cache/remote info and examples Mar 15, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 15, 2021 07:28 Inactive
jorgeorpinel added a commit that referenced this pull request Mar 17, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 20:12 Inactive
jorgeorpinel added a commit that referenced this pull request Mar 17, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 20:25 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 20:28 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 20:55 Inactive
@jorgeorpinel
Copy link
Contributor Author

jorgeorpinel commented Mar 17, 2021

@shcheklein on the example titles, I changed them again after thinking about it:

add:

  • Example: External data (to-cache)
  • Example: --to-remote usage (--to-remote)

Word "Adding" left out for consistency with previous examples e.g. "Example: Single file" but no strong opinion. Could also start with "Tracking".

import:

  • Example: --to-remote usage (--to-remote)

Cc @isidentical @dberenbaum in case you have ideas 🙂

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 21:06 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 21:21 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 17, 2021 21:24 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 18, 2021 01:31 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 18, 2021 01:54 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-jorge-udzxd3nasv3bydv9 March 19, 2021 04:56 Inactive
jorgeorpinel added a commit that referenced this pull request Mar 19, 2021
@jorgeorpinel jorgeorpinel mentioned this pull request Mar 19, 2021
1 task
jorgeorpinel added a commit that referenced this pull request Mar 19, 2021
jorgeorpinel added a commit that referenced this pull request Mar 19, 2021
jorgeorpinel added a commit that referenced this pull request Mar 19, 2021
@jorgeorpinel jorgeorpinel merged commit 4d5bd5f into master Mar 19, 2021
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.

5 participants