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

fetch/push/status: not handling config from other revisions #9754

Open
1 of 3 tasks
efiop opened this issue Jul 24, 2023 · 2 comments
Open
1 of 3 tasks

fetch/push/status: not handling config from other revisions #9754

efiop opened this issue Jul 24, 2023 · 2 comments
Labels
A: data-sync Related to dvc get/fetch/import/pull/push bug Did we break something? p2-medium Medium priority, should be done, but less important

Comments

@efiop
Copy link
Contributor

efiop commented Jul 24, 2023

When branches have wildly different remote setups, those configs are not taken into account during fetch/push/status -c --all-tags/branches/etc

Example:

#!/bin/bash

set -e
set -x

rm -rf mytest
mkdir mytest
cd mytest

mkdir remote1
mkdir remote2
remote1="$(pwd)/remote1"
remote2="$(pwd)/remote2"

mkdir repo
cd repo
git init
dvc init
git commit -m "init"
git branch branch1
git branch branch2

git checkout branch1
echo foo > foo
dvc add foo
dvc remote add -d myremote1 $remote1
dvc push
git add .gitignore foo.dvc .dvc/config
git commit -m "foo"

git checkout branch2
echo bar > bar
dvc add bar
dvc remote add -d myremote2 $remote2
dvc push
git add .gitignore bar.dvc .dvc/config
git commit -m "bar"

git checkout main
rm -rf .dvc/cache
dvc fetch --all-branches
tree .dvc/cache  # will show 0 files

Studio uses real git checkout to collect objects and has been doing that for years as a workaround, but I couldn't find an issue in dvc yet.

To fix this we should make config part of Index(same as stages, outs, etc are, don't confuse with DataIndex) and use it to build Index.data. This is the easiest to do in dvc fetch because it is using Index.data already, but might require temporary workarounds for push/status -c like manually triggering config reloading in brancher or something.

@efiop efiop added bug Did we break something? A: data-sync Related to dvc get/fetch/import/pull/push labels Jul 24, 2023
efiop added a commit to efiop/dvc that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to iterative#9754
efiop added a commit to efiop/dvc that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to iterative#9754
efiop added a commit to efiop/dvc that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to iterative#9754
efiop added a commit to efiop/dvc that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to iterative#9754
efiop added a commit to efiop/dvc that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to iterative#9754
efiop added a commit that referenced this issue Jul 24, 2023
Config is a more immutable thing and using `repo` is confusing and might be error
prone in the bigger picture.

Related to #9754
efiop added a commit to efiop/dvc that referenced this issue Jul 25, 2023
Related iterative#9754 and fixes `dvc fetch`.
efiop added a commit that referenced this issue Jul 25, 2023
Related #9754 and fixes `dvc fetch`.
@dberenbaum
Copy link
Collaborator

When branches have wildly different remote setups

Do we respect any of the per-revision remote config? Or we always use the remote config from the workspace?

@efiop
Copy link
Contributor Author

efiop commented Jul 25, 2023

@dberenbaum Always using the remote config from the workspace. Not respecting per-revision remote configs at all 🙁 Now fixed for fetch though.

@dberenbaum dberenbaum added the p2-medium Medium priority, should be done, but less important label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: data-sync Related to dvc get/fetch/import/pull/push bug Did we break something? p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

No branches or pull requests

2 participants