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

Task/rfr 1188 handle upload conflict #179

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hb0
Copy link
Contributor

@hb0 hb0 commented Feb 11, 2025

The issues were:

  • missing index on google collection, multiple attachments metadata were allowed to be stored
  • no handling when a file already exists

This PR:

  • creates the missing indices for measurement and attachment metadata documents
  • identifies when the collector tries to store an already stored file => inform client that file already exists

I checked manually that:

  • In this instance, both copies of the uploaded file were equally large and contain the number of bytes expected (metadata.uploadLength): Both in the metadata uploadLength and the google cloud storage object linked in the metadata have the same and expected size, so we do not have an issue with "inclomplete uploads".
  • The client interprets "CONFLICT" as "UPLOAD_SUCCESSFUL" and continues with the next upload.

See code for more details.

After this PR was merged

  • TODO: Remove duplicate documents before we deploy this to Staging (which tries to enable the unique key indices) @hb0
  • Then deployed to Staging, which enables unique indices @hb0
  • Then try to upload the remaining files with the Pixel 7 and see if upload works now @muthenberg

@hb0 hb0 self-assigned this Feb 11, 2025
@hb0 hb0 requested a review from muthenberg February 11, 2025 12:38
Copy link
Contributor Author

@hb0 hb0 left a comment

Choose a reason for hiding this comment

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

pre-reviewed

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.

1 participant