fix: Added region and signature for S3 bucket

[farhaanbukhsh]

For a region based bucket boto3 needs to know about region and signature which edx-ora2 doesn't know about.

TL;DR - Currently edx-ora2 doesn't pick up AWS region or aws signature version and because of that boto3 generates a faulty S3 URL. The boto3 documentation for s3 signature v4 also shows the boto3 client need to know the signature version.

JIRA: None

What changed?

• Config to add AWS configuration for S3

When I was trying to upload the file in S3 bucket in the me-south-1 region, I kept on getting

The reason was boto3 client doesn't use S3_SIGNATURE=s3v4 and region value here in edx-ora2 while the same config was working properly on discussions.

Developer Checklist

• ✅ Reviewed the release process
• ❌ Translations and JS/SASS compiled
• ❌ Bumped version number in setup.py and package.json

Testing Instructions

• None since it just adds configs

Reviewer Checklist

Collectively, these should be completed by reviewers of this PR:

• I've done a visual code review
• I've tested the new functionality

[openedx-webhooks]

Thanks for the pull request, @farhaanbukhsh! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (20626e4) 94.98% compared to head (0318118) 94.98%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1928   +/-   ##
=======================================
  Coverage   94.98%   94.98%           
=======================================
  Files         154      154           
  Lines       17056    17058    +2     
  Branches     1611     1611           
=======================================
+ Hits        16201    16203    +2     
  Misses        641      641           
  Partials      214      214           

Flag	Coverage Δ
unittests	94.98% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
openassessment/__init__.py	100.00% <100.00%> (ø)
openassessment/fileupload/backends/s3.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kaustavb12]

👍

• I tested this: Tested upload file to S3 works in sandbox
• I read through the code
• I checked for accessibility issues
• Includes documentation
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

[itsjeyd]

Thanks @farhaanbukhsh and @kaustavb12!

Looks like this PR is ready for engineering review. I changed its status on the contributions board accordingly.

[itsjeyd]

Hi @mattcarter, if you could help get this PR lined up for engineering review by the Aurora team, that would be great!

[farhaanbukhsh]

@mattcarter Gentle ping on the PR.

[itsjeyd]

label: core contributor

[itsjeyd]

Hi @mattcarter, just checking in to see if you had a chance to line this PR up for engineering review by Aurora?

[Agrendalath]

@farhaanbukhsh, we may also want to bump the version in openassessment/__init__.py and package.json.

[farhaanbukhsh]

@farhaanbukhsh, we may also want to bump the version in openassessment/init.py and package.json.

Thank you for the review @Agrendalath will make the changes :)

[itsjeyd]

@farhaanbukhsh It looks like we'll be able to proceed with a CC review here soon (Slack ref). Could you please address @Agrendalath's comments in the coming days to get ready for that?

[farhaanbukhsh]

@itsjeyd @Agrendalath Thank you for the feedback, I have made the changes and pushed the commit, :) please have a look.

[Agrendalath]

The changes look good to me 👍🏻

[itsjeyd]

@pomegranited Following confirmation from the Aurora team (@mattcarter) that this PR could be merged after CC review I'm assigning this PR to you.

For additional context, a decision was made recently on how to use the assignee field in the context of OSPR management going forward -- see 1b in this section on the Open edX wiki.

Thanks for volunteering! 🙂

CC @mphilbrick211 @hurtstotouchfire @nedbat @farhaanbukhsh @Agrendalath

[pomegranited]

Thanks for this fix @farhaanbukhsh !

I left a few minor suggestions, but I've tested this in both scenarios below, and it works fine. So I'll approve when the nits are addressed.

• uploading to bucket in us-east-1 with defaults left at None
• uploading to bucket in us-east-2 with signature_version = "s3v4", region_name = "us-east-2"

[pomegranited]

nit: master is currently at '5.1.0', so:

Suggested change

	__version__ = '5.0.5'
	__version__ = '5.1.1'

[farhaanbukhsh]

@pomegranited I have updated the code with your suggestions. Thanks a lot for the review :)

[pomegranited]

👍 perfect, thank you @farhaanbukhsh !

Since I've got permission from Aurora to merge, I'll do that now, and tag a new release.

• I tested this on my devstack:
  • left defaults at None, and tested file uploads to a bucket in us-east-1.
  • set signature_version = "s3v4", region_name = "us-east-2" and tested file uploads to a bucket in that region.
• I read through the code
• I checked for accessibility issues N/A
• Includes documentation N/A

[openedx-webhooks]

@farhaanbukhsh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.