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

remote: separate remote/cache/tree into distinct concepts #3882

Closed
6 tasks done
pmrowla opened this issue May 27, 2020 · 2 comments
Closed
6 tasks done

remote: separate remote/cache/tree into distinct concepts #3882

pmrowla opened this issue May 27, 2020 · 2 comments
Assignees
Labels
enhancement Enhances DVC p1-important Important, aka current backlog of things to do refactoring Factoring and re-factoring

Comments

@pmrowla
Copy link
Contributor

pmrowla commented May 27, 2020

Currently, Remote classes can contain methods that fall into three categories:

  • remote operations - indexing/gc/etc
  • cache operations - save/checkout/link/etc
  • tree operations - exists/isfile/isdir/upload/download/etc

Having all of these methods grouped into a single Remote class causes issues when we are dealing with paths that can come from different namespaces (trees). In particular, LocalRemote was originally written with the assumption that all paths for local remote or local cache operations are always filesystem paths, which is no longer the case given that we now need to support GitTree objects (via brancher and erepos).

Ex. LocalRemote.exists(path) is used in different places as a replacement for os.path.exists() to test if path exists in a local remote, if path exists in the local cache directory, and if path exists in a DVC repo, even though these are distinct operations.

Tasks:

@pmrowla pmrowla added enhancement Enhances DVC refactoring Factoring and re-factoring p1-important Important, aka current backlog of things to do labels May 27, 2020
@pmrowla pmrowla self-assigned this May 27, 2020
@efiop
Copy link
Contributor

efiop commented May 28, 2020

remote operations - upload/download/etc

Maybe upload/download are actually related to the tree, but push/pull(aka fetch)/status are definitely remote concepts that will use upload/download in respective trees.

@pmrowla
Copy link
Contributor Author

pmrowla commented Jun 16, 2020

Some minor cleanup left w/improving remote function naming, but after #4041 this refactor is pretty much done for now.

#4050 has been filed as a potential follow up issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p1-important Important, aka current backlog of things to do refactoring Factoring and re-factoring
Projects
None yet
Development

No branches or pull requests

2 participants