Skip to content

Conversation

@efiop
Copy link
Contributor

@efiop efiop commented Jan 25, 2021

When we collect cache for pull/fetch or run status -c, we implicitly try to fetch the dir cache entry from the remote. And that's the behaviour that is expected from RepoTree too. There have been multiple times where we've tried to use Repo.repo_tree just to realise that it tries to fetch automatically, which is often not needed.

This PR is more of a POC that tries to get rid of fetch flag and the implicit fetching during walk() that it implies in favor of just fetching stuff explicitly where we really need it (repo has fetch method just for that already).

Need to check performance and also 2 webdav tests fail now because they don't have open() support, and Tree doesn't fetch things automatically, as expected by the api code.

Depends on #5307

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

Comment on lines +103 to +109
Copy link
Contributor Author

@efiop efiop Jan 25, 2021

Choose a reason for hiding this comment

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

Strictly speaking this is not really needed, as cache.save() will fetch everything anyway, but I've added this for now because logic around this (e.g. tests) expects specific exceptions and also because this might be faster than save(), as we didn't optimize it yet. Need to check perf first, maybe we can remove it right now.

@efiop efiop force-pushed the erepo branch 3 times, most recently from 5e5922a to 67172a8 Compare January 26, 2021 21:55
Comment on lines +442 to +443
kw = kwargs.copy()
kw.pop("download_callback", None)
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 temporary, we'll start computing hash on-the-fly in the followup PRs.

@efiop efiop changed the title [WIP] RepoTree: get rid if implicit fetching RepoTree: get rid if implicit fetching Jan 27, 2021
@efiop efiop merged commit 969a85e into treeverse:master Jan 27, 2021
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.

1 participant