Skip to content

RepoTree: get rid if implicit fetching #5324

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

Merged
merged 1 commit into from
Jan 27, 2021
Merged

RepoTree: get rid if implicit fetching #5324

merged 1 commit into from
Jan 27, 2021

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
used = repo.used_cache(
[os.fspath(path_info)],
force=True,
jobs=jobs,
recursive=True,
)
cb(repo.cloud.pull(used, jobs))
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 iterative: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