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 inconsistent s3 metadata field names #1360

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Conversation

fvankrieken
Copy link
Contributor

@fvankrieken fvankrieken commented Jan 2, 2025

Resolves #1350

When attempting to update the config of an existing version of a recipe dataset, we get a weird "signature does not match" error. This occurs on calling put_object when the object already exists. Googling has been entirely unhelpful - this error seems to occur when people have bad credentials. This makes me think that this might be something to do with Digital Ocean - I can't imagine there are that many folks using boto3 with DO, which would be a possible reason for lack of other people having this issue Issue was with the metadata I was attempting to push. Due to a bug in our code, when pulling metadata and then pushing again (and "merging" with standard generated metadata), there ended up being a both a "date-created" field and "date_created" field, with differing values. Resolving this resolved the issue

This logic maybe needs a little reworking in general - not great that I had to change multiple literals.

Actions (all using dev bucket)

These were run in order

Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.41%. Comparing base (b667b99) to head (2bc5dc5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dcpy/connectors/edm/publishing.py 50.00% 1 Missing ⚠️
dcpy/connectors/edm/recipes.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1360      +/-   ##
==========================================
- Coverage   70.42%   70.41%   -0.01%     
==========================================
  Files         115      115              
  Lines        5978     5976       -2     
  Branches      695      695              
==========================================
- Hits         4210     4208       -2     
  Misses       1622     1622              
  Partials      146      146              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fvankrieken
Copy link
Contributor Author

fvankrieken commented Jan 2, 2025

The fact that this is failing is bizarre. Those jobs have no problem deleting the existing config, but then two lines later when pushing the new config fail. I wonder if something is weird with trying to keep the existing metadata - like there's something internal that I'm trying to set that DO/S3 doesn't like.

Maybe this is it - looks like somehow I'm trying to set date_created twice here

Resolved!

@fvankrieken fvankrieken force-pushed the fvk-put-object-weirdness branch from e80b725 to 292e471 Compare January 2, 2025 18:20
@fvankrieken fvankrieken changed the title delete s3 config if exists before updating fix inconsistent s3 metadata field names Jan 2, 2025
Copy link
Contributor

@alexrichey alexrichey left a comment

Choose a reason for hiding this comment

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

good catch!

@fvankrieken fvankrieken merged commit 4998595 into main Jan 2, 2025
20 of 22 checks passed
fvankrieken added a commit that referenced this pull request Jan 2, 2025
@fvankrieken fvankrieken deleted the fvk-put-object-weirdness branch January 2, 2025 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Scheduled Action Failure - Ingest - 📁 Open Data Routine Updates
4 participants