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

fix(filemanager): ingest constraints #136

Merged
merged 7 commits into from
Mar 8, 2024

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Mar 6, 2024

Closes #131

Changes

  • Make version_id required with a default value as the 'null' string.
  • Ensure that only one row is returned from the main current_objects select query when reordering events.
  • Various test fixes:
    • Add tests for missing created/deleted events, missing sequencer values, and multiple matching rows for reordering.
    • Fix permutation tests and add smaller non-ignored tests.

@@ -24,6 +24,9 @@ create table s3_object (
bucket text not null,
-- The key of the object.
key text not null,
-- The version id of the object. A 'null' string is used to indicate no version id. This matches logic in AWS which
-- also returns 'null' strings.
version_id text not null default 'null',
Copy link
Member Author

@mmalenic mmalenic Mar 7, 2024

Choose a reason for hiding this comment

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

I know this looks weird, but I think that it is okay because AWS specifies that when the version_id is the string 'null', then versioning is not enabled: https://docs.aws.amazon.com/AmazonS3/latest/userguide/versioning-workflows.html. I.e. effectively, a version_id is always present, and AWS uses 'null' when it's disabled.

There is an alternative, and that's to have another constraint that has nulls not distinct on the version_id only, e.g. versioning_id_unique, and then adding additional insert queries that target versioning_id_unique for de-duplication. I feel that this adds more repetitive code, although I'm not sure which is the better approach.

Copy link
Member

Choose a reason for hiding this comment

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

First option LGTM, weird but it's documented... I'd probably add that docs url in the comment itself for reference.

@mmalenic mmalenic requested a review from brainstorm March 7, 2024 00:06
@mmalenic mmalenic self-assigned this Mar 7, 2024
@mmalenic mmalenic added filemanager an issue relating to the filemanager fix labels Mar 7, 2024
@@ -24,6 +24,9 @@ create table s3_object (
bucket text not null,
-- The key of the object.
key text not null,
-- The version id of the object. A 'null' string is used to indicate no version id. This matches logic in AWS which
-- also returns 'null' strings.
version_id text not null default 'null',
Copy link
Member

Choose a reason for hiding this comment

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

First option LGTM, weird but it's documented... I'd probably add that docs url in the comment itself for reference.

@mmalenic mmalenic merged commit 6d28e31 into main Mar 8, 2024
2 checks passed
@mmalenic mmalenic deleted the fix/filemanage-ingest-constraint branch March 8, 2024 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filemanager an issue relating to the filemanager fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filemanager: assuming Object Removed events occur when an object is overwritten
2 participants