Data sharing manager bug and scaling fixes #970
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Quality in report should be optional Fixed table expiry and expiry query Push S3 uses distributed map Fix map iter issue Add concurrency Comment out lambda example executions
Distributed map policy is not a child of the state machine object so must be explicitly added to the nag suppressions
There was a problem hiding this comment.
Pull Request Overview
This PR addresses several bug fixes and scaling improvements for the Data Sharing Manager, including optional quality fields in reports, table expiry query fixes, and enhanced S3 push functionality with distributed map support.
- Removed main block examples across multiple lambda modules to clean up production code.
- Made the quality field in the data summary report optional to support legacy metadata.
- Updated the deployment stack to attach a distributed map policy to the state machine role for improved S3 push operations.
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/workload/stateless/stacks/data-sharing-manager/lambdas/get_workflow_from_portal_run_id_py/get_workflow_from_portal_run_id.py | Main block examples now commented out. |
| lib/workload/stateless/stacks/data-sharing-manager/lambdas/get_s3_destination_and_source_uri_mappings_py/get_s3_destination_and_source_uri_mappings.py | Added new event keys (count_only, pagination_index) and list sorting logic. |
| lib/workload/stateless/stacks/data-sharing-manager/lambdas/get_files_list_from_portal_run_id_py/get_files_list_from_portal_run_id.py | Main block examples now commented out. |
| lib/workload/stateless/stacks/data-sharing-manager/lambdas/generate_presigned_urls_for_data_objects_py/generate_presigned_urls_for_data_objects.py | Main block examples now commented out. |
| lib/workload/stateless/stacks/data-sharing-manager/interface/data_sharing_api/models/package.py | Adjusted timezone handling for expiration checks and updated package retrieval for expiration state. |
| lib/workload/stateless/stacks/data-sharing-manager/ecs/tasks/generate_data_summary_report/data_summary_reporting_tools/src/data_summary_reporting_tools/models.py | Made the 'quality' field optional to improve compatibility with legacy reports. |
| lib/workload/stateless/stacks/data-sharing-manager/deploy/stack.ts | Updated docker asset naming and added distributed map policy for S3 push state machine. |
Files not reviewed (2)
- lib/workload/stateless/stacks/data-sharing-manager/step_functions_templates/packaging_sfn_template.asl.json: Language not supported
- lib/workload/stateless/stacks/data-sharing-manager/step_functions_templates/push_s3_data_sfn_template.asl.json: Language not supported
Comments suppressed due to low confidence (1)
lib/workload/stateless/stacks/data-sharing-manager/lambdas/get_s3_destination_and_source_uri_mappings_py/get_s3_destination_and_source_uri_mappings.py:146
- The commented example uses 'paginationList', which is inconsistent with the code that expects 'paginationIndex'. Update the example to match the correct parameter name for clarity.
# "paginationList": [0, 23],
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Quality stat in report lims should be optional (makes a lot of metadata table from previous years compatible)
Fixed table expiry and expiry query
Push S3 uses distributed map
Fix map iter issue
Add concurrency
Comment out lambda example executions