Skip to content

BB-1744: Bump pymongo version and fix course export issue - #196

Merged
giovannicimolin merged 1 commit into
opencraft-release/ironwood.2from
giovanni/bb-1744-upgrade-pymongo
Nov 7, 2019
Merged

BB-1744: Bump pymongo version and fix course export issue#196
giovannicimolin merged 1 commit into
opencraft-release/ironwood.2from
giovanni/bb-1744-upgrade-pymongo

Conversation

@giovannicimolin

@giovannicimolin giovannicimolin commented Oct 31, 2019

Copy link
Copy Markdown

This commit upgrades the version of pymongo from 2.x to 3.x, removing usages to deprecated functions usage and fixing tests where necessary.

This version of pymongo supports MongoDB 2.x all the way up to 4.2, and this ensures that the platform will be able to run on a supported MongoDB version in the next release.

(cherry picked from commit 9b69d5a62684dc3d9d86b7e71081bebd432bd7d9)

See discussion and testing instructions on upstream ticket https://github.com/edx/edx-platform/pull/21197.

Sandboxes:
Mongo 3.2: https://console.opencraft.com/instance/17030/
Mongo 3.6: https://console.opencraft.com/instance/17070/
Mongo 4.0: https://console.opencraft.com/instance/17080/

Reviewers:

This commit upgrades the version of pymongo from 2.x to 3.x, removing usages to deprecated functions usage and fixing tests where necessary.

This version of pymongo supports MongoDB 2.x all the way up to 4.2, and this ensures that the platform will be able to run on a supported MongoDB version in the next release.

(cherry picked from commit 9b69d5a62684dc3d9d86b7e71081bebd432bd7d9)

@lgp171188 lgp171188 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@giovannicimolin, the changes look good. I have left a few comments/questions. Can you check those out? I know this PR only has the backported changes from the upstream PR, but I left comments on the actual changes since I didn't review that,

Comment thread common/lib/xmodule/xmodule/modulestore/mongo/base.py
Comment thread common/lib/xmodule/xmodule/course_metadata_utils.py
@lgp171188

lgp171188 commented Nov 6, 2019

Copy link
Copy Markdown

@giovannicimolin, while testing this on the sandbox set up for MongoDB 3.6, I found some exceptions in the celery worker logs during the import, which may not be directly related to the changes made in this PR.

Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/celery/worker/consumer.py", line 465, in on_task_received
    strategies[type_](message, body,
KeyError: u'openedx.core.djangoapps.bookmarks.tasks.update_xblock_cache'

and

Traceback (most recent call last):
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/contentstore/content.py", line 440, in generate_thumbnail
    with Image.open(source) as image:
  File "/edx/app/edxapp/venvs/edxapp/local/lib/python2.7/site-packages/PIL/Image.py", line 2687, in open
    % (filename if filename else fp))
IOError: cannot identify image file <StringIO.StringIO instance at 0x7f7d0a8f6830>

FYI.

@giovannicimolin

Copy link
Copy Markdown
Author

@lgp171188 I've debugged the issues:
I didn't dig to deep on the first one, but it's unrelated to these changes.

I've managed to reproduce the issue on the generate_thumbnail function: the assets that fail are corrupt images from the demonstration course (maybe this is intentional to test the import reliability?).
corrupt_images

@lgp171188

Copy link
Copy Markdown

@giovannicimolin, I have resolved all my questions/comments after checking your responses. I also ran the python tests locally in my devstack and the few errors/failures that I saw were not related to the changes made in this PR. 👍

@lgp171188 lgp171188 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

  • I tested this in the sandboxes and didn't find any issues. I also ran the tests locally and didn't see any errors or failures related to the changes in this PR.
  • I read through the code
  • I checked for accessibility issues NA
  • Includes documentation NA

@giovannicimolin
giovannicimolin merged commit b48ced8 into opencraft-release/ironwood.2 Nov 7, 2019
@giovannicimolin
giovannicimolin deleted the giovanni/bb-1744-upgrade-pymongo branch November 7, 2019 17:08
Kelketek pushed a commit that referenced this pull request Sep 8, 2020
Sustaining/security fixes 4

(cherry picked from commit e637704)
shimulch pushed a commit that referenced this pull request Sep 9, 2020
toxinu pushed a commit that referenced this pull request Sep 18, 2020
Sustaining/security fixes 4

(cherry picked from commit e637704)
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.

2 participants