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: tre egress store data updated S3 return #1054

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

maghirardelli
Copy link
Contributor

Issue #, if available:

Description of changes:
S3 introduced a new attribute in their listObjectsV2 method that contains the checksum algorithm for S3 objects in Feb 2022. The test did not expect the attribute. The attribute is empty as we are not specifying a checksum algorithm during put calls.

Checklist:

AS review ticket id:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@maghirardelli maghirardelli self-assigned this Oct 12, 2022
@maghirardelli maghirardelli requested a review from a team as a code owner October 12, 2022 15:38
@codecov
Copy link

codecov bot commented Oct 12, 2022

Codecov Report

Merging #1054 (1f9a011) into develop (3e9d28a) will increase coverage by 7.62%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1054      +/-   ##
===========================================
+ Coverage    51.93%   59.56%   +7.62%     
===========================================
  Files          238      126     -112     
  Lines        14508     9224    -5284     
  Branches      2361     1475     -886     
===========================================
- Hits          7535     5494    -2041     
+ Misses        6106     3288    -2818     
+ Partials       867      442     -425     
Impacted Files Coverage Δ
lib/user/user-service.js 64.61% <0.00%> (-6.10%) ⬇️
src/helpers/api.js 0.00% <0.00%> (-0.86%) ⬇️
src/parts/dashboard/Dashboard.js
src/models/forms/CreateAwsAccountForm.js
src/parts/helpers/By.js
src/models/forms/CreateStudy.js
src/models/forms/AddProjectForm.js
src/models/aws-accounts/Budget.js
src/parts/files/FileDropZone.js
src/parts/studies/CreateStudy.js
... and 106 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e9d28a...1f9a011. Read the comment docs.

@@ -70,6 +70,7 @@ describe('Create URL scenarios', () => {
StorageClass: 'STANDARD',
projectId: 'TRE-Project',
workspaceId: 'ea8e5286-45ca-402a-8c98-7d4495f646e2',
ChecksumAlgorithm: [], // new attribute introduced in Feb 2022 https://aws.amazon.com/blogs/aws/new-additional-checksum-algorithms-for-amazon-s3/
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the comment is necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sanket suggested it so people that look at this test later know why it was needed.

@maghirardelli maghirardelli merged commit a80aa7e into develop Oct 12, 2022
@maghirardelli maghirardelli deleted the fix-tre-integ-tests branch October 12, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants