Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove symmath from edx-platform #29869

Merged
merged 7 commits into from
Feb 10, 2022
Merged

Conversation

Carlos-Muniz
Copy link
Contributor

@Carlos-Muniz Carlos-Muniz commented Feb 3, 2022

Description

This PR removes symmath from common/lib/, all removable mentions of symmath within edx-platform and upgrades the requirements to no longer use a non-existent symmath.

This is the second part of BOM-2581.
Symmath has been added to openedx-calc alongside calc in openedx-calc PR #38.

Testing Instructions

The code has been tested:

  • Manually:
    • A custom dev image of lms/cms was built using this branch via tutor
    • Tutor was started in dev mode using these images
    • While these containers were running, the version of openedx-calc was checked to be 3.0.1, and that symmath was not in common/lib/
    • In the CMS GUI (Studio), a new course was formed, and a new symbolic math problem was initiated.
    • Answers that were mathematically correct, but symbolically different were entered into the sample problem
  • Locally: Pytest via Tutor
  • Online: All checks have passed

Additional information

There is an issue with pip-tools using pip version >=22.0.0. Pip is currently constrained to <20.3.4, and so this shouldn't be an issue, but this may still be an issue.

There is currently an issue with the package m2r, which was causing make upgrade to fail. The dependency mistune has been constrained to <2.0.0 in order for everything to work as it once did.

In the near future, these may be fixed.

@Carlos-Muniz Carlos-Muniz self-assigned this Feb 3, 2022
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch 2 times, most recently from 8aa03e9 to fbb00dc Compare February 3, 2022 22:03
@Carlos-Muniz Carlos-Muniz marked this pull request as draft February 4, 2022 12:07
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch from 8743c0f to 3c8b3b2 Compare February 4, 2022 16:47
@Carlos-Muniz Carlos-Muniz marked this pull request as ready for review February 4, 2022 17:04
@jmbowman
Copy link
Contributor

jmbowman commented Feb 4, 2022

Cool! Could you drop a note in https://openedx.atlassian.net/browse/BOM-2581 once this is merged so we can close it out? There are more tasks like this in that ticket's parent epic if you want to keep going. ^_^

Oh, and there's a new pip-tools release that should resolve the issue you noted: https://github.com/jazzband/pip-tools/releases/tag/6.5.0

common/lib/capa/capa/responsetypes.py Outdated Show resolved Hide resolved
requirements/constraints.txt Outdated Show resolved Hide resolved
requirements/edx/local.in Show resolved Hide resolved
scripts/verify-dunder-init.sh Show resolved Hide resolved
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch from 2bee665 to 72da685 Compare February 7, 2022 18:26
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Did you test this out manually? The ## Test instructions section of the PR description is a great place to communicate whether & how you manually verified that your code works -- sometimes I use it less as "test instructions" and more as "test explanation".

scripts/post-pip-compile.sh Outdated Show resolved Hide resolved
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch from 2b10b1b to eb95153 Compare February 7, 2022 19:40
@kdmccormick
Copy link
Member

Code looks good! Only blocker for me is this question: #29869 (review)

@Carlos-Muniz
Copy link
Contributor Author

Code looks good! Only blocker for me is this question: #29869 (review)

The code has been tested:

  • Manually: openedx-calc 3.0.1 has been installed in a local env and made sure that it was able to be imported as it is in the code.
  • Locally: Pytest via Tutor
  • Online: All checks have passed.

@kdmccormick
Copy link
Member

@Carlos-Muniz Could you go into LMS/Studio and confirm that a symbolic math problem renders and can be responded to?

@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch 2 times, most recently from f3c7861 to ffbfe19 Compare February 9, 2022 15:25
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Pending tests, looks good to me 👍🏻

Carlos Muniz added 7 commits February 10, 2022 14:55
This allows common/lib/capa/capa/responsetypes.py to not have to
change.
scripts/post-pip-compile.sh uses common/lib/symmath/ as its sample
directory to clean up after using pip-tools. Since symmath has been
removed, xmodule, the largest of the directories in common/lib/ will
replace it.
@Carlos-Muniz Carlos-Muniz force-pushed the Carlos-Muniz/symmath-removal branch from ffbfe19 to d5a41ec Compare February 10, 2022 20:11
@Carlos-Muniz Carlos-Muniz merged commit a6718f9 into master Feb 10, 2022
@Carlos-Muniz Carlos-Muniz deleted the Carlos-Muniz/symmath-removal branch February 10, 2022 20:31
@Carlos-Muniz
Copy link
Contributor Author

Closes openedx/axim-engineering#65

@jdmulloy
Copy link
Contributor

@Carlos-Muniz @kdmccormick I can't figure out why, but something in this broke the edxapp build on GoCD. This is the ansible output where it's failing. We may need to revert this.

`TASK [edxapp : code sandbox | Install base sandbox requirements and create sandbox virtualenv] ***
fatal: [10.4.11.90]: FAILED! => {
"changed": false,
"cmd": [
"/edx/app/edxapp/venvs/edxapp-sandbox/bin/pip3",
"install",
"-i",
"https://pypi.python.org/simple",
"--exists-action",
"w",
"-r",
"/edx/app/edxapp/edx-platform/requirements/edx-sandbox/py38.txt"
]
}

MSG:

stdout: Looking in indexes: https://pypi.python.org/simple
Obtaining file:///edx/app/edxapp/edx-platform/common/lib/sandbox-packages (from -r /edx/app/edxapp/edx-platform/requirements/edx-sandbox/py38.txt (line 7))
Collecting cffi==1.15.0
Downloading cffi-1.15.0-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (446 kB)
Collecting chem==1.2.0
Downloading chem-1.2.0-py3-none-any.whl (24 kB)
Collecting click==7.1.2
Downloading click-7.1.2-py2.py3-none-any.whl (82 kB)
Collecting cryptography==36.0.1
Downloading cryptography-36.0.1-cp36-abi3-manylinux_2_24_x86_64.whl (3.6 MB)
Collecting cycler==0.11.0
Downloading cycler-0.11.0-py3-none-any.whl (6.4 kB)
Collecting decorator==4.4.2
Downloading decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
Collecting joblib==1.1.0
Downloading joblib-1.1.0-py2.py3-none-any.whl (306 kB)
Collecting kiwisolver==1.3.2
Downloading kiwisolver-1.3.2-cp38-cp38-manylinux_2_5_x86_64.manylinux1_x86_64.whl (1.2 MB)
Collecting lxml==4.5.0
Downloading lxml-4.5.0-cp38-cp38-manylinux1_x86_64.whl (5.6 MB)
Collecting markupsafe==2.0.1
Downloading MarkupSafe-2.0.1-cp38-cp38-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (30 kB)
Collecting matplotlib==3.3.4
Downloading matplotlib-3.3.4-cp38-cp38-manylinux1_x86_64.whl (11.6 MB)
Collecting mpmath==1.2.1
Downloading mpmath-1.2.1-py3-none-any.whl (532 kB)
Collecting networkx==2.5.1
Downloading networkx-2.5.1-py3-none-any.whl (1.6 MB)
Collecting nltk==3.7
Downloading nltk-3.7-py3-none-any.whl (1.5 MB)
Collecting numpy==1.16.6
Downloading numpy-1.16.6.zip (5.1 MB)
Collecting openedx-calc==3.0.1
Downloading openedx_calc-3.0.1-py3-none-any.whl (34 kB)
Collecting pillow==9.0.1
Downloading Pillow-9.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (4.3 MB)
Collecting pycparser==2.21
Downloading pycparser-2.21-py2.py3-none-any.whl (118 kB)
Collecting pyparsing==3.0.7
Downloading pyparsing-3.0.7-py3-none-any.whl (98 kB)
Collecting python-dateutil==2.4.0
Downloading python_dateutil-2.4.0-py2.py3-none-any.whl (175 kB)
Collecting random2==1.0.1
Downloading random2-1.0.1.zip (21 kB)
Collecting regex==2022.1.18
Downloading regex-2022.1.18-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (764 kB)
Collecting scipy==1.7.3
Downloading scipy-1.7.3-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (39.3 MB)
Collecting six==1.16.0
Downloading six-1.16.0-py2.py3-none-any.whl (11 kB)
Collecting sympy==1.6.2
Downloading sympy-1.6.2-py3-none-any.whl (5.8 MB)
Collecting tqdm==4.62.3
Downloading tqdm-4.62.3-py2.py3-none-any.whl (76 kB)
Building wheels for collected packages: numpy, random2
Building wheel for numpy (setup.py): started
Building wheel for numpy (setup.py): still running...
Building wheel for numpy (setup.py): finished with status 'done'
Created wheel for numpy: filename=numpy-1.16.6-cp38-cp38-linux_x86_64.whl size=9934057 sha256=78575335ee1e7b9ac3b9a0556268394a6a87d170c94da58f6a142ba0ac4aad84
Stored in directory: /edx/app/edxapp/venvs/edxapp-sandbox/.cache/pip/wheels/22/6f/a6/069db0c95f9bb6b73e07da014891c58460fef1eb84f49576f0
Building wheel for random2 (setup.py): started
Building wheel for random2 (setup.py): finished with status 'done'
Created wheel for random2: filename=random2-1.0.1-py3-none-any.whl size=12070 sha256=d6cc470f3c506b36072961e8357d8eec3521f4ba058eaf145f7d7a727a764047
Stored in directory: /edx/app/edxapp/venvs/edxapp-sandbox/.cache/pip/wheels/6e/ca/f2/082dec051ffcaec249ae491b8c90e305726a7390274682b4a4
Successfully built numpy random2
Installing collected packages: tqdm, six, regex, pycparser, numpy, mpmath, joblib, click, sympy, scipy, python-dateutil, pyparsing, pillow, nltk, markupsafe, lxml, kiwisolver, decorator, cycler, cffi, sandbox-packages, random2, openedx-calc, networkx, matplotlib, cryptography, chem
Running setup.py develop for sandbox-packages

:stderr: ERROR: Command errored out with exit status 1:
command: /edx/app/edxapp/venvs/edxapp-sandbox/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"'; file='"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(file);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, file, '"'"'exec'"'"'))' develop --no-deps
cwd: /edx/app/edxapp/edx-platform/common/lib/sandbox-packages/
Complete output (3 lines):
running develop
running egg_info
error: [Errno 13] Permission denied
----------------------------------------
ERROR: Command errored out with exit status 1: /edx/app/edxapp/venvs/edxapp-sandbox/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"'; file='"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(file);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, file, '"'"'exec'"'"'))' develop --no-deps Check the logs for full command output.
WARNING: You are using pip version 20.3.4; however, version 22.0.3 is available.
You should consider upgrading via the '/edx/app/edxapp/venvs/edxapp-sandbox/bin/python -m pip install --upgrade pip' command.`

@kdmccormick
Copy link
Member

@jdmulloy weird! This is totally safe to revert if needed -- no data migrations or anything.

@Carlos-Muniz and I can take a look at a fix tomorrow.

@pshiu
Copy link
Contributor

pshiu commented Feb 11, 2022

Thanks, @kdmccormick, @Carlos-Muniz! Reverting now to see if we can get a green build.

@kdmccormick
Copy link
Member

This line in the logs jumped out at me:

:stderr: ERROR: Command errored out with exit status 1:
command: /edx/app/edxapp/venvs/edxapp-sandbox/bin/python -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"'; file='"'"'/edx/app/edxapp/edx-platform/common/lib/sandbox-packages/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(file);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, file, '"'"'exec'"'"'))' develop --no-deps
cwd: /edx/app/edxapp/edx-platform/common/lib/sandbox-packages/
Complete output (3 lines):
running develop
running egg_info
error: [Errno 13] Permission denied

Seems like something changed with ./common/lib/sandbox-packages.

Lo and behold: https://github.com/openedx/edx-platform/pull/29869/files#diff-b637ba9a442d8914ab88f5231e76fa60adc6d862fc13fdcaff4bb27329edd4beL7-R7. Notice the additional -e that this PR adds.

@kdmccormick
Copy link
Member

@Carlos-Muniz ^

@Carlos-Muniz
Copy link
Contributor Author

Great catch @kdmccormick!

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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.

6 participants