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

[ENG-338] [OATHPIT] Throw S3 into the Oath Pit #388

Merged

Conversation

cslzchen
Copy link
Contributor

@cslzchen cslzchen commented Nov 8, 2019

Ticket

https://openscience.atlassian.net/browse/ENG-338

Purpose

Enable and update the S3 provider for aiohttp 3.

Changes

It is unknown why aiohttp-0.18.x stores HTTP response header names in all-uppercase (e.g. 'CONTENT-LENGTH', 'ETAG', etc.). aiohttp-3.5.x now uses the more popular title case (e.g. 'Content-Length', 'ETAG', etc.). This breaks the old implementation for class S3FileMetadataHeaders whose calling self.raw['ETAG'] throws Key Error. The class and related tests has been updated.

Side effects

No

QA Notes

Here is a list of tests that I have done locally, all of which have passed. Extra notes and quirks are added under each list if there is any. In short, S3 behaves as expected.

  • Getting metadata for a file and folder

    • This is automatic when you list files and expand folders
  • Downloading (20B, 1 MB, 11 MB, 88 MB, 264MB)

  • Uploading (20B, 1 MB, 11 MB, 88 MB, 264MB one-by-one and together)

    • Both contiguous and chunked
  • DAZ (a folder containing five files of 20B, 1 MB, 11 MB, 88 MB, 264MB)

  • Delete a file and a folder; delete multiple files and folders

  • Folder creation and deletion

  • Renaming files and folders

    • Similar to ownCloud, after renaming a folder or file (successfully), clicking on the renamed folder triggers the following error.

click-on-an-renamed-file-or-folder

  • Intra move and copy

    • Between two components connected to the same account and the same bucket.
  • Inter move and copy

    • Between S3 and OSFStorage
    • Between S3 and another 3rd-party (ownCloud in this case)
  • Comments persist with moves

  • Revisions

  • Updating a file

  • Project root is storage root vs. a subfolder

    • N / A: s3 only allow bucket root as OSF project root

Deployment Notes

TBD

@coveralls
Copy link

coveralls commented Nov 8, 2019

Coverage Status

Coverage increased (+4.6%) to 61.524% when pulling 38a2e92 on cslzchen:feature/oathpit-s3 into 61107f3 on CenterForOpenScience:feature/oathpit.

@cslzchen cslzchen changed the title ENG-338] [OATHPIT] Throw S3 into the Oath Pit [ENG-338] [OATHPIT] Throw S3 into the Oath Pit Nov 11, 2019
Copy link
Contributor Author

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Local tests have been done and QA notes updated. Please rebase and fix conflicts.

waterbutler/providers/s3/metadata.py Show resolved Hide resolved
tests/providers/s3/test_metadata.py Show resolved Hide resolved
It is unknown why aiohttp 0.18.x stores HTTP response header names
in all-uppercase (e.g. CONTENT-LENGTH). aiohttp 3.5.x now uses the
more popular title case (e.g. Content-Length) which breaks the old
implementation (i.e. Key Error). The class `S3FileMetadataHeaders`
has been updated to fix this issue. Tests are fixed as well.
@cslzchen cslzchen merged commit 38a2e92 into CenterForOpenScience:feature/oathpit Nov 14, 2019
@cslzchen cslzchen deleted the feature/oathpit-s3 branch January 13, 2020 18:34
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.

None yet

2 participants