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

repo: drop_data_index: delete all first level nodes #10090

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Nov 13, 2023

This resulted in outdated fetch index cache and misc errors that are cleared by removing site_cache_dir manually. So because of the latter fact, also bumping site_cache_dir salt here to force refresh.

Likely related #10030

More tests coming separately.

@efiop efiop added the bug Did we break something? label Nov 13, 2023
@efiop efiop force-pushed the test-push-version_aware branch from ae75299 to 49bc92a Compare November 13, 2023 09:02
@efiop efiop marked this pull request as ready for review November 13, 2023 09:09
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Files Coverage Δ
dvc/repo/__init__.py 94.11% <66.66%> (-0.50%) ⬇️

... and 38 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

@efiop efiop force-pushed the test-push-version_aware branch from 49bc92a to 82da75a Compare November 13, 2023 09:15
@efiop efiop marked this pull request as draft November 13, 2023 09:19
@efiop efiop marked this pull request as ready for review November 13, 2023 09:22
@efiop efiop merged commit e9f2da4 into iterative:main Nov 13, 2023
17 of 18 checks passed
@dberenbaum
Copy link
Collaborator

@efiop Could you explain more so I understand the changes? Was it not deleting any of the nodes before? Why do only the first-level nodes get dropped?

@efiop
Copy link
Contributor Author

efiop commented Nov 13, 2023

@dberenbaum It was only deleting tree prefix. Logical thing to do here instead is delete_node(()) to delete root node, but sqltrie has an assumption that root always exists (and probably rightfully so), so we have to just delete all first level entries instead (deleting a node means that you are breaking the traversal and descendants become orphaned, so we don't need to waste time deleting them here right now). There are a few small things like that about root that bother me, but didn't want to address them here right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Did we break something?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants