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

Improve methods to cleanup collections and buckets (GSI-902) #119

Merged
merged 9 commits into from
Jul 16, 2024

Conversation

Cito
Copy link
Member

@Cito Cito commented Jul 16, 2024

PR #115 changed the behavior of the empty_buckets method in an attempt to make it more similar to empty_collections.

Before, it used to only empty the buckets that were populated using the fixture. Now, it empties all existing buckets that can be accessed. This has the advantage that the fixture does not need to track which buckets were populated and can also clean buckets that were not populated using the fixture. However, this can now also empty buckets in the S3 instance that are not part of the application or tests and not supposed to be touched.

Therefore, this PR adds parameters for specifying the collections and buckets to be emptied in addition to the parameter that already allowed specifying which collections and buckets should be excluded.

In addition, these parameters are now made keyword-only to avoid mistakes when calling these methods.

Another question in this context was how to handle objects that were not created using hexkit and therefore can have names that do not comply with the validation rules for ids implemented in hexkit. These will cause validation errors when hexkit tries to delete them when emptying the containing bucket. This could happen for instance with object names that contain forward slashes or start with a dot. In these cases, we intentionally let the error bubble up instead of trying to delete these objects despite the error or silently ignore them, because they usually indicate a problem that should be made apparent.

However, this PR also facilitates using different validation rules for object names, by not hard coding the regular expression used for validation, but exposing it as an attribute that can be overridden. Also, the PR relaxes the validation a little bit by allowing underscores and longer object names. Forward slashes are still not allowed by default, though.

@coveralls
Copy link

coveralls commented Jul 16, 2024

Pull Request Test Coverage Report for Build 9956350161

Details

  • 41 of 47 (87.23%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.06%) to 91.697%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/hexkit/providers/mongodb/testutils.py 10 12 83.33%
src/hexkit/providers/s3/testutils/_fixtures.py 25 29 86.21%
Totals Coverage Status
Change from base Build 9839054397: -0.06%
Covered Lines: 1756
Relevant Lines: 1915

💛 - Coveralls

@Cito Cito requested a review from TheByronHimes July 16, 2024 10:50
Copy link
Member

@TheByronHimes TheByronHimes left a comment

Choose a reason for hiding this comment

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

Looks good on the whole. I only saw a few small things.

src/hexkit/protocols/objstorage.py Outdated Show resolved Hide resolved
src/hexkit/providers/s3/testutils/_fixtures.py Outdated Show resolved Hide resolved
src/hexkit/providers/s3/testutils/_fixtures.py Outdated Show resolved Hide resolved
tests/integration/test_mongodb.py Show resolved Hide resolved
Cito and others added 3 commits July 16, 2024 13:52
Co-authored-by: Byron Himes <[email protected]>
Co-authored-by: Byron Himes <[email protected]>
@Cito Cito requested a review from TheByronHimes July 16, 2024 12:03
@Cito Cito merged commit 1460024 into main Jul 16, 2024
8 checks passed
@Cito Cito deleted the feature/improve-empty-buckets-GSI-902 branch July 16, 2024 12:04
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.

3 participants