Skip to content

Upgrade to Mongo 3.6 - #505

Merged
melvinsoft merged 5 commits into
appsembler/tahoe/developfrom
maxi/upgrade-to-mongo-3.6
Dec 26, 2019
Merged

Upgrade to Mongo 3.6#505
melvinsoft merged 5 commits into
appsembler/tahoe/developfrom
maxi/upgrade-to-mongo-3.6

Conversation

@melvinsoft

Copy link
Copy Markdown

No description provided.

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.
@melvinsoft melvinsoft changed the title (WIP) Maxi/upgrade to mongo 3.6 Maxi/upgrade to mongo 3.6 Dec 23, 2019

@OmarIthawi OmarIthawi 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.

@melvinsoft That's amazing! Please add the tests to tox.ini:

common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py
common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py
lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py
lms/djangoapps/verify_student/tests/test_services.py

@OmarIthawi OmarIthawi changed the title Maxi/upgrade to mongo 3.6 Upgrade to Mongo 3.6 Dec 24, 2019
@thraxil

thraxil commented Dec 24, 2019

Copy link
Copy Markdown

Exciting. The commit notes say "This version of pymongo supports MongoDB 2.x all the way up to 4.2". We might even be able to move to 4.x in the foreseeable future.

@melvinsoft

Copy link
Copy Markdown
Author

@OmarIthawi Thanks for the review, I added the test you requested but then I deleted one: lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py since is testing new code we don't have yet, and more intended for python 3 rather than 2.

But tests are still failing, new fails are related to Access Control Backend:

py27-lms run-test: commands[3] | lms/lib/tests/test_access_control_backends.py openedx/core/djangoapps/appsembler openedx/core/djangoapps/site_configuration/tests/test_tahoe_changes.py openedx/core/djangoapps/user_api/accounts/tests/test_utils.py::CompletionUtilsTestCase
ERROR: InvocationError for command could not find executable lms/lib/tests/test_access_control_backends.py

I invested about three hours until I reached this point, can you please take a quick look? If not we should move forward without the new tests, and do it as a follow up work.

What do you think?

@OmarIthawi

OmarIthawi commented Dec 24, 2019

Copy link
Copy Markdown

Thanks @melvinsoft! tox.ini now has syntax errors and I've fixed them.

TL; DR;

In general I think in tests for the edx-platform we probably want to take things slowly and steer clear from rabbit holes. But now the tests are fixed, we don't need to worry about it.

Even in the best case scenario, we don't want all tests to run, maybe 99% at max, this is to make it possible for us to deviate from Open edX in a healthy way until we make plugins or APIs.

Thank you again!

@OmarIthawi OmarIthawi 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.

Let's get those trailing slashes to avoid breaking the tests

Comment thread tox.ini Outdated
Comment thread tox.ini Outdated
Comment thread tox.ini Outdated
@OmarIthawi
OmarIthawi force-pushed the maxi/upgrade-to-mongo-3.6 branch from 5eb818d to 7babed6 Compare December 24, 2019 17:36

@OmarIthawi OmarIthawi 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.

Thanks @melvinsoft! I've fixed the tox.ini and cleaned the branch (I also did a push --force). I'll merge once tests passes.

Note that this might break devstack (right?) and we'd might to upgrade MongoDB there as far as I know.

@melvinsoft

Copy link
Copy Markdown
Author

@OmarIthawi Thanks for all the help here. I'm merging this now.

@melvinsoft
melvinsoft merged commit 224c7dc into appsembler/tahoe/develop Dec 26, 2019
@melvinsoft

Copy link
Copy Markdown
Author

@OmarIthawi Why do you think is gonna break devstack? because the authSource and replicaSet parameters in the connection?

@OmarIthawi
OmarIthawi deleted the maxi/upgrade-to-mongo-3.6 branch December 26, 2019 18:51
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.

4 participants