overhangio / openedx-scorm-xblock

SCORM XBlock for Open edX
GNU Affero General Public License v3.0
37 stars 48 forks source link

Avoid file seek when saving extracted files #9

Closed shadinaif closed 3 years ago

shadinaif commented 3 years ago

We are facing the following error because files are uploaded to Google Storage

  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/xblock/runtime.py", line 1037, in handle
    results = handler(request, suffix)
  File "/edx/app/edxapp/venvs/edxapp/src/openedxscorm/openedxscorm/scormxblock.py", line 223, in studio_submit
    self.extract_package(package_file)
  File "/edx/app/edxapp/venvs/edxapp/src/openedxscorm/openedxscorm/scormxblock.py", line 280, in extract_package
    dest_path, scorm_zipfile.open(zipinfo.filename),
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/files/storage.py", line 54, in save
    return self._save(name, content)
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/storages/backends/gcloud.py", line 159, in _save
    file.blob.upload_from_file(content, size=content.size,
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/files/base.py", line 59, in _get_size
    self._size = self._get_size_from_underlying_file()
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/django/core/files/base.py", line 49, in _get_size_from_underlying_file
    pos = self.file.tell()
UnsupportedOperation: seek

This PR is to fix it by avoiding partial reads that need seek operation

regisb commented 3 years ago

Thanks for the PR @shadinaif! If I understand correctly, the gcloud backend needs to access the size of the individual files stored in the zipfile, but the file-like objects created by open do not have a seek method: https://docs.python.org/3.6/library/zipfile.html#zipfile.ZipFile.open At first I thought that this would create an issue with large uploaded zip-files, but in fact only the individual files stored in the zipfile will be uncompressed this way.

shadinaif commented 3 years ago

Thank you for the fast response @regisb . I've applied the requested code change