Skip to content

remote: finish separating RemoteTree methods#3946

Merged
efiop merged 15 commits into
treeverse:masterfrom
pmrowla:remote-tree
Jun 4, 2020
Merged

remote: finish separating RemoteTree methods#3946
efiop merged 15 commits into
treeverse:masterfrom
pmrowla:remote-tree

Conversation

@pmrowla
Copy link
Copy Markdown
Contributor

@pmrowla pmrowla commented Jun 3, 2020

  • ❗ I have followed the Contributing to DVC checklist.

  • 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

  • Moves remainder of tree-related methods out of Remote classes into the RemoteTree classes
  • Implements tree.walk_files() for remotes that did not already have the method
  • Makes list_cache_paths() use tree.walk_files() and behave the same for all cloud remotes except local/ssh

Related to #3882.

@pmrowla pmrowla self-assigned this Jun 3, 2020
Comment thread dvc/remote/azure.py Outdated
Copy link
Copy Markdown
Contributor

@efiop efiop Jun 4, 2020

Choose a reason for hiding this comment

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

Would it make sense to make these just Azure or AzureTree and move them to dvc/tree/azure.py instead of continually thinking about them as remotes? Not saying it should be in this PR, but maybe in the followups soon?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it's worth considering if we are going to start using these tree's for non-remote/cache related use cases (like workspaces).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We do that already for external outputs scenario :) Workspaces are a just a potential better abstraction for that.

But ok, let's merge and then do that later.

pmrowla added 3 commits June 4, 2020 14:39
* list_cache_paths() now uses tree.walk_files() for all remotes except
  local/ssh
@pmrowla pmrowla marked this pull request as ready for review June 4, 2020 07:03
@pmrowla pmrowla changed the title [WIP] remote: finish separating RemoteTree methods remote: finish separating RemoteTree methods Jun 4, 2020
@pmrowla pmrowla requested a review from efiop June 4, 2020 07:16
@efiop efiop merged commit 74bd813 into treeverse:master Jun 4, 2020
@pmrowla pmrowla deleted the remote-tree branch June 5, 2020 01:33
@pmrowla pmrowla mentioned this pull request Jun 5, 2020
3 tasks
efiop pushed a commit that referenced this pull request Jun 5, 2020
* change DvcParser to print subcommand help

* add tests for subcommand help

* docstring refactoring

* remote: finish separating RemoteTree methods (#3946)

* remote: move upload()/download() into tree

* remote: move path_info/path_cls into tree

* remote.azure: finish moving methods into tree

* remote.gs: finish moving methods into tree

* remote.hdfs: finish moving methods to tree

* remote.http: finish moving methods into tree

* remote.oss: finish moving methods into tree

* remote.s3: finish moving methods into tree

* remote.ssh: finish moving methods into tree

* remote.gdrive: finish moving methods into tree

* tests: update remote unit tests

* tests: update func tests

* remote: use walk_files() for all remotes

* list_cache_paths() now uses tree.walk_files() for all remotes except
  local/ssh

* fix DS warnings

* bugfixes

* Unify metrics behavior with dirs (#3935)

* run: disable directories for -m/-M flags

* run: reformatted string literals

* run: simplified metrics check

* tests: func: run: dirs as metrics outs

* tests: func: run: reformatted string literals

* tests: use pytest

Co-authored-by: Ruslan Kuprieiev <ruslan@iterative.ai>

* refactor/test multistage load for params and outputs (#3949)

* refactor multistage load for params and outputs

* tests: load params

* tests: output loading from pipeline file

* fix test

* fix typo in name

* split params load

* rename params func s/inject_values/fill_values

* fix tests

* simplify loads_params and output.load_from_pipeline

* address @pared's suggestions for tests

* remote: fix build errors (#3957)

* remote: fix remaining gdrive build issues (#3959)

* tests: fix gdrive build

* remote.gdrive: fix walk_files prefix relpath

* change DvcParser to print subcommand help

* add tests for subcommand help

* docstring refactoring

* change tests to pytest-style

* remove gettext

Co-authored-by: Peter Rowlands (변기호) <peter@pmrowla.com>
Co-authored-by: nik123 <kodenkonikita@gmail.com>
Co-authored-by: Ruslan Kuprieiev <ruslan@iterative.ai>
Co-authored-by: Saugat Pachhai <suagatchhetri@outlook.com>
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.

2 participants