dvc: Repo(..., rev=smth) is counter-intuitive or broken

[Suor]

For example repo.add() will browse using currently existing .dvcignore files. repo.pull/push() will work against master not rev.

Should we hide it? Or handle all of that? Or implement everything not using non-default rev Repo objects?

• [ ] either fix or drop rev
• [ ] remove assert rev is None in dvc.api

[Suor]

P.S. the examples above are not exhaustive, e.g. Config also ignores rev and who knows what else.

[efiop]

@Suor Are you sure? pull/push will use self.tree(which is set to rev), so they should do the right thing. Config will indeed not work. Either way, need to check it.

[Suor]

@efiop .push()/pull() use brancher and brancher only uses self.tree, if no kwargs are passed. So it was not as I described, but still fishy.

Still the point stands - do we really need repos with non-default rev?

[efiop]

We do for erepos, to be able to collect and pull stuff. But there might be a nicer way to do that.

[Suor]

I would say that we may break someone workflow if make Repo reread config for each branch :) Some older branch may have no remote config. And when it will have a different one it could be quite surprising for a user that his files are pushed to two different remotes.

[efiop]

I would say that we may break someone workflow if make Repo reread config for each branch :) Some older branch may have no remote config. And when it will have a different one it could be quite surprising for a user that his files are pushed to two different remotes.

Yep, need to give that one a thought. On one hand, it seems like it would be a correct approach none the less, but on the other maybe the current behavior is actually what we want (at least with some explicit option). The least we could do is to check if other branches have different remotes to the one in the current workspace and prompt or raise a warning/error.

For the record: Agreed in a private discussion to remove rev from Repo.__init__ and use self.tree with brancher in a more explicit way.