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

Resolving issue #375 StorageClass header is not added to upload request #398

Merged
merged 5 commits into from
May 24, 2018

Conversation

pzavadska
Copy link
Contributor

Added possibility to set StorageClass in ObjectMetadata (setStorageClass(StorageClass storageClass))
Save value into DB (TransferRecord). Table adjusted.
Adjusted tests to test also set StorageClass.

…pload request

Added possibility to set StorageClass in ObjectMetadata (setStorageClass(StorageClass storageClass))
Save value into DB (TransferRecord). Table adjusted.
Adjusted tests to test also set StorageClass.
@mutablealligator
Copy link
Contributor

Hi @petra, Thank you for submitting a pull request. We are reviewing it.

@mutablealligator mutablealligator added the s3 Issues with the AWS Android SDK for Simple Storage Service (S3). label Jan 16, 2018
@mutablealligator
Copy link
Contributor

Sorry for the delayed response. Can you remove the unused import of StorageClass from TransferRecord.java? There seem to be some extra whitespaces in some of the code changes. Can you remove them?

Removed unused import from TransferRecord.java
Reformatted all files changed by this pull request with built-in "AWS Mobile SDK for Android" formatter
@pzavadska
Copy link
Contributor Author

@kvasukib I've removed the unused import and reformatted all touched classes with your "AWS Mobile SDK for Android" formatter. Seems like the code was not formatted properly before my changes thus some additional formatting was changed not only whitespaces.

@codecov-io
Copy link

Codecov Report

Merging #398 into master will increase coverage by <.01%.
The diff coverage is 14.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #398      +/-   ##
==========================================
+ Coverage    6.01%    6.01%   +<.01%     
==========================================
  Files        5910     5910              
  Lines      230145   230166      +21     
  Branches    54038    54041       +3     
==========================================
+ Hits        13846    13852       +6     
- Misses     215141   215156      +15     
  Partials     1158     1158
Impacted Files Coverage Δ
...ors/s3/transferutility/TransferDatabaseHelper.java 0% <ø> (ø) ⬆️
...obileconnectors/s3/transferutility/UploadTask.java 0% <0%> (ø) ⬆️
...leconnectors/s3/transferutility/TransferTable.java 0% <0%> (ø) ⬆️
...econnectors/s3/transferutility/TransferDBUtil.java 0% <0%> (ø) ⬆️
...econnectors/s3/transferutility/TransferRecord.java 0% <0%> (ø) ⬆️
...om/amazonaws/services/s3/model/ObjectMetadata.java 74.57% <66.66%> (+3.27%) ⬆️

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 0c3eae4...93649ac. Read the comment docs.

@stale
Copy link

stale bot commented May 16, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the closing soon Issue will auto-close if there is no additional activity within 7 days. label May 16, 2018
@pzavadska
Copy link
Contributor Author

pzavadska commented May 16, 2018

Does anybody care to merge the pull request after 4 months?

@stale stale bot removed the closing soon Issue will auto-close if there is no additional activity within 7 days. label May 16, 2018
@mutablealligator mutablealligator self-assigned this May 16, 2018
@mutablealligator
Copy link
Contributor

Hi @padamuscinova, Sorry for the delay. I am working on it.

@mutablealligator
Copy link
Contributor

mutablealligator commented May 18, 2018

I was able to reproduce the issue. The issue is that the S3 putObject method extracts the storage class from the put object request and adds it as an Http header to the underlying request object. However, the TransferUtility does not have a provision to accept the storage class directly and we rely on the custom user metadata which is added to the ObjectMetadata. The UploadTask in TransferUtility does not add the storage class to the put object request.

Special case for GLACIER: https://docs.aws.amazon.com/AmazonS3/latest/dev/storage-class-intro.html#sc-glacier

@mutablealligator
Copy link
Contributor

Approving the PR. Will merge it in the upcoming release.

@minbi minbi merged commit 3d9a8ce into aws-amplify:master May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s3 Issues with the AWS Android SDK for Simple Storage Service (S3).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants