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

output: use remote kwarg instead of odb #6600

Merged
merged 4 commits into from
Sep 14, 2021
Merged

output: use remote kwarg instead of odb #6600

merged 4 commits into from
Sep 14, 2021

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Sep 13, 2021

Fixes #6596

@efiop efiop requested a review from a team as a code owner September 13, 2021 10:03
@efiop efiop changed the title output: use remote kwarg instead of odb [WIP] output: use remote kwarg instead of odb Sep 13, 2021
@@ -849,7 +849,8 @@ def get_dir_cache(self, **kwargs):
try:
objects.check(self.odb, obj)
except FileNotFoundError:
remote = self.repo.cloud.get_remote_odb(self.remote)
if self.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.

This is getting irritating, will need to look into getting rid of get_dir_cache soon. 😞

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't -r <remote> be of higher priority?

Copy link
Member

@skshetry skshetry Sep 13, 2021

Choose a reason for hiding this comment

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

My second choice would be to skip the remotes not matching with -r <remote>.

Copy link
Member

Choose a reason for hiding this comment

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

Though, for dir cache, I would not care that much. 🤷🏼

Copy link
Contributor Author

@efiop efiop Sep 13, 2021

Choose a reason for hiding this comment

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

remote: has higher prioirity than -r, as documented in https://github.com/iterative/dvc.org/pull/2761/files . Similar how dvc imported remotes are not overriden by dvc pull -r

dvc/output.py Outdated Show resolved Hide resolved
Co-authored-by: Dave Berenbaum <dave@iterative.ai>
@efiop efiop changed the title [WIP] output: use remote kwarg instead of odb output: use remote kwarg instead of odb Sep 14, 2021
@efiop efiop added the bugfix fixes bug label Sep 14, 2021
@efiop efiop merged commit dae0825 into master Sep 14, 2021
@efiop efiop deleted the efiop-patch-1 branch September 14, 2021 15:34
@efiop
Copy link
Contributor Author

efiop commented Sep 14, 2021

ci failure is unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pull -r remote: Disregards specified http remote
3 participants