Skip to content

Conversation

@ceciliastevens
Copy link
Contributor

Updates the file download methods to use EDS instead of PDS. Also removes the old fallback to downloading from backup archives, as that is no longer used now that we're on EDS.

@ceciliastevens
Copy link
Contributor Author

ceciliastevens commented May 5, 2025

Builds are failing because the mock-servers used for integration testing don't yet handle the EDS url in the key-value store.

Edit: after discussion, it looks like the best thing to do here is just to remove those tests, as they're no longer relevant.

@ceciliastevens ceciliastevens marked this pull request as draft May 5, 2025 21:17
@ceciliastevens ceciliastevens marked this pull request as ready for review May 6, 2025 16:21
def get_download_token(
self, event_id, device_id, file_path, file_sha256, timestamp
):
def get_download_token(self, downloadRequestUrl):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only customer using this as far as we know? Changing the signature is technically a breaking change - but if its currently broken and/or no ones using it, then its probably okay.
Maybe we could add details about how to adopt the fix in the changelog?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is only used internally and isn't included in the documentation. I suppose it's possible that someone could have been using it directly, but I doubt it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note to the changelog.

@tora-kozic
Copy link
Contributor

If we're planning to release this right after merging, feel free to do the version bump and the changelog release date in the same PR

@ceciliastevens ceciliastevens merged commit 1ba5fcd into main May 6, 2025
5 of 6 checks passed
@ceciliastevens ceciliastevens deleted the INTEG-2918/file-download-fix branch May 6, 2025 16:36
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