openedx / RecommenderXBlock

edX: An XBlock to recommend resources to other students, written by Daniel Li, under my supervision
GNU Affero General Public License v3.0
5 stars 16 forks source link

feat: valid S3 expiration time #75

Closed ArturGaspar closed 10 months ago

ArturGaspar commented 11 months ago

Description

Recommender XBlock will currently generate invalid URLs if used with S3 backend. This fixes that.

Using the filesystem backend should be unaffected as it ignores the timeout parameter (https://github.com/openedx/django-pyfs/blob/v3.4.1/djpyfs/djpyfs.py#L116).

Testing instructions

  1. Configure openedx-django-pyfs to use S3
      DJFS = {
          'type': 's3fs',
          'bucket': AWS_STORAGE_BUCKET_NAME,
          'prefix': 'pyfs/',
          # Note that explicitly passing credentials requires openedx-django-pyfs >= 3.4.1
          'aws_access_key_id': AWS_ACCESS_KEY_ID,
          'aws_secret_access_key': AWS_SECRET_ACCESS_KEY
      }
  2. Add a resource in the XBlock, include a screenshot
  3. See that the screenshot is displayed
  4. Configure openedx-django-pyfs to use the local filesystem
      DJFS = {
          'type': 'osfs',
          'directory_root': '/tmp/pyfs',
          'url_root': '/static/djpyfs'
      }
  5. Add a resource in the XBlock, include a screenshot
  6. See that the screenshot is displayed

Other information

Private-ref: https://tasks.opencraft.com/browse/BB-8077

openedx-webhooks commented 11 months ago

Thanks for the pull request, @ArturGaspar! 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:

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.

mphilbrick211 commented 11 months ago

Hi @ArturGaspar - is this all set for review?

ArturGaspar commented 11 months ago

@mphilbrick211 Yes.

mphilbrick211 commented 10 months ago

Hi @openedx/2u-arbi-bom! This is ready for review.

awais786 commented 10 months ago

@ArturGaspar Please bump the version.

ArturGaspar commented 10 months ago

@awais786 Done.

openedx-webhooks commented 10 months ago

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