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

Data Sync Operation updates - mem leak fix and introduction to results response #18

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

rpmcginty
Copy link
Collaborator

@rpmcginty rpmcginty commented Nov 9, 2024

  • cache botocore config
  • updating result returned from data sync ops
  • formatting and adding more tests

What's in this Change?

  • caching of botocore config
  • introducing DataSyncResult and BatchDataSyncResult
    • summarizes data that is transferred
    • allows to conditionally update
  • for batch jobs, we add the ability for partial success of transfers (by default this is disabled)

Primarily meant to address the following bug - https://alleninstitute.atlassian.net/browse/OCSDV-354

This also introduces a new approach to gathering results during data sync. Previously the data sync operations returned nothing and the lambda handlers would simply return the requests in the response. For a few reasons, this is not great and so I have introduced a more informative and cohesive result object that summarizes data sync operations based on bytes transferred, files transferred and for the batch case, the number of succeeded and failed tasks.

Connected PRs

Testing

  • unit tests
  • e2e tests in merscope analysis dev

@rpmcginty rpmcginty force-pushed the bugfix/data-sync-memory-leak branch from 68c2537 to f2be6fb Compare November 9, 2024 00:11
@njmei njmei self-requested a review November 12, 2024 19:39
@rpmcginty rpmcginty force-pushed the bugfix/data-sync-memory-leak branch from 89d2074 to b40d6a8 Compare November 12, 2024 20:37
@rpmcginty rpmcginty marked this pull request as ready for review November 12, 2024 20:37
@rpmcginty rpmcginty merged commit 1e50004 into main Nov 12, 2024
4 checks passed
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.

2 participants