read_batch set include_deleted to false by default when reading a version #1419
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
willdealtry
approved these changes
Mar 15, 2024
grusev
pushed a commit
that referenced
this pull request
Nov 25, 2024
…sion (#1419) Fixes #1385 Similar to a normal read, `batch_read` now sets `include_deleted` to false when looking for an index key via `find_index_key_for_version_id` - removed `test_batch_read_metadata_multi` as `self._versions.items()` has versions that have already been deleted, the test tries to lookup the deleted versions and tries to asserts that they are found - removed `test_batch_read_snapshot` as the `VersionStoreComparison` creates a test sequence where it first deletes a symbol, then with `test_batch_read_snapshot` checks that the versions in the snapshot match the versions read with `batch_read` which is not the expected behaviour because deleted versions will not be returned by batch read. - similarly removed `test_batch_read_tombstoned_version_via_snapshot` as, with `batch_read(symbol, as_of=[version])` , we shouldn't expect to read a snapshotted version that has been deleted. #### Checklist <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1385
Similar to a normal read,
batch_readnow setsinclude_deletedto false when looking for an index key viafind_index_key_for_version_idtest_batch_read_metadata_multiasself._versions.items()has versions that have already been deleted, the test tries to lookup the deleted versions and tries to asserts that they are foundtest_batch_read_snapshotas theVersionStoreComparisoncreates a test sequence where it first deletes a symbol, then withtest_batch_read_snapshotchecks that the versions in the snapshot match the versions read withbatch_readwhich is not the expected behaviour because deleted versions will not be returned by batch read.test_batch_read_tombstoned_version_via_snapshotas, withbatch_read(symbol, as_of=[version]), we shouldn't expect to read a snapshotted version that has been deleted.Checklist
Checklist for code changes...