build: add missing files to PyPI distribution (v3.3.0) #140

kdmccormick · 2022-10-03T15:41:35Z

Description

The existing PyPI distribution (edx-codejail 3.2.0) was missing several files that made it unsuitable for installation by edx-platform:

It was missing root scripts: proxy_main.py and memory_stress.py.
It was missing the apparmor-profiles and sudoers-files directories.
It wasn't missing any Python modules under codejail, but if any sub-packages had been added to codejail, they would have been omitted from the distribution. This could have bitten someone in the future.

To fix, we modify setup.py and add a MANIFEST.in, in line with the pattern established by the cookiecutter repo:

https://github.com/openedx/edx-cookiecutters/blob/master/python-template/%7B%7Bcookiecutter.placeholder_repo_name%7D%7D/setup.py

This wasn't affecting edx-platform because edx-platform currently does a GitHub-based editable installation (-e git+https://...) for codejail. However, we'd like to move away from those editable installations as a they are slow and cannot be kept up-to-date with make upgrade.

Bump from 3.2.0 to 3.3.0.

Testing

Clone codejail at this branch and navigate into it. Create and activate a virtualenv. Then run:

pip install -r requirements/pip.txt
python setup.py sdist bdist_wheel
cd dist
tar xzf edx-codejail-3.3.0.tar.gz
cd edx-codejail-3.3.0

This will drop you into the directory that pip will actually install into edx-platform's virtual environment. Ensure that it contains proxy_main.py and that the codejail directory contains all the source modules other than the test modules.

Some Context

There are three different ways we install packages into edx-platform, in decreasing order of desirability:

As pre-built wheels from PyPI (the vast majority; everything in base.in)
As wheels from GitHub (git+https://...)
As editable (aka "dev-mode") packages from GitHub (-e git+https://)

edx-platform installs codejail using method (3) currently. I'd tried to change it over to method (2) but it failed on edX.org with:

Traceback (most recent call last):
  File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 931, in _extract_context
    safe_exec(
  File "/usr/lib/python3.8/contextlib.py", line 75, in inner
    return func(*args, **kwds)
  File "/edx/app/edxapp/edx-platform/xmodule/capa/safe_exec/safe_exec.py", line 170, in safe_exec
    exec_fn(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/safe_exec.py", line 154, in safe_exec
    res = jail_code.jail_code(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/jail_code.py", line 318, in jail_code
    status, stdout, stderr = run_subprocess_fn(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 91, in run_subprocess_through_proxy
    six.reraise(*last_exception)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/six.py", line 719, in reraise
    raise value
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 76, in run_subprocess_through_proxy
    status, stdout, stderr, log_calls = deserialize_out(proxy_stdout.rstrip())
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 46, in deserialize_out
    return deserialize_in(bstr.decode('utf8'))
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 42, in deserialize_in
    return ast.literal_eval(ustr)
  File "/usr/lib/python3.8/ast.py", line 59, in literal_eval
    node_or_string = parse(node_or_string, mode='eval')
  File "/usr/lib/python3.8/ast.py", line 47, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 1
    /edx/app/edxapp/venvs/edxapp/bin/python: can't open file '/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/proxy_main.py': [Errno 2] No such file or directory
    ^
SyntaxError: invalid syntax

because proxy_main.py was not included in the wheel. This PR fixes that, thus allowing us to install codejail into edx-platform using method (2) or (1).

kdmccormick · 2022-10-03T15:47:24Z

@nedbat could I trouble you for a review on this?

nedbat

A few minor corrections, then good to go.

.github/workflows/pypi-publish.yml

.gitignore

nedbat · 2022-10-04T02:09:00Z

MANIFEST.in

+include requirements/*
+include apparmor-profiles/*
+include sudoers-file/*
+recursive-include {{cookiecutter.sub_dir_name}} *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg


cookiecutter here seems wrong, and I don't think any of these file types are in this repo.

Whoops, this was just bad copy-pasting on my part:

recursive-include {{cookiecutter.sub_dir_name}} ...

the cookiecutter would've templated that to:

recursive-include codejail ...

Anyway, I'll just remove that line since it's not relevant to us.

nedbat · 2022-10-04T02:10:37Z

MANIFEST.in

@@ -0,0 +1,7 @@
+include CHANGELOG.rst


There is no CHANGELOG.rst file, though there should be.

I think I missed the memo that we are doing changelogs now. I see them in the cookiecutter but I don't know when that happened. It's not clear to me why we would ever do changelogs and github releases and conventional commits and PR templates. It seems like we're asking developers for a lot of overlapping information.

I'm going to just delete this line for now, but I'll poke arch-bom and pitch the idea of dropping one of those (hopefully GH releases) so we can focus on the things we really care about developers doing.

I poked arch-bom: https://discuss.openedx.org/t/changelogs-and-reducing-developer-busywork/8435

The existing PyPI distribution (edx-codejail 3.2.0) was missing several files that made it unsuitable for installation by edx-platform: * It was missing root scripts: proxy_main.py and memory_stress.py. * It was missing the apparmor-profiles and sudoers-files directories. * It wasn't missing any Python modules under codejail, but *if* any sub-packages had been added to codejail, they would have been omitted from the distribution. This could have bitten someone in the future. To fix, we modify setup.py and add a MANIFEST.in, in line with the pattern established by the cookiecutter repo: https://github.com/openedx/edx-cookiecutters/blob/master/python-template/%7B%7Bcookiecutter.placeholder_repo_name%7D%7D/setup.py This wasn't affecting edx-platform because edx-platform currently does a GitHub-based editable installation (`-e git+https://...`) for codejail. However, we'd like to move away from those editable installations as a they are slow and cannot be kept up-to-date with `make upgrade`. Bump from 3.2.0 to 3.3.0.

UsamaSadiq · 2022-10-06T10:35:28Z

Thanks @kdmccormick for this effort.
We'd tried using approach(1) at the end of the last year but it had failed at that time and I couldn't figure out the failure at that time. Now, after your updated changes, this has unblocked our issues as well.
A bundle of thanks for this PR.

kdmccormick · 2022-10-07T22:00:41Z

No problem, glad to help!

aht007 · 2022-10-28T06:57:20Z

@kdmccormick @nedbat
Coming back to this PR as it was supposed to resolve the issue with proxy_main.py issue but unfortunately it didn't.
Yesterday I tried installing codejail version 3.3.1 from PyPI. Meanwhile It worked fine on sandboxes(which is still a dilemma for me and the team), it was still broken on stage server. Fortunately I had paused the pipeline and hence the error didn't go past the dev server to production server.
During our investigation we found out that proxy_main.py was being accessed from this path /edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/proxy_main.py in the stage server which in itself seems wrong and according to @UsamaSadiq this might be coming from this line of code that being said we are still unclear why this issue is not happening with the sandboxes.
Also when installing codejail from PyPI, the scripts(proxy_main.py and others) end up in the bin directory of our virtualenv whereas other code files go to the lib directory. But when installing using editable mode there is nothing in both lib and bin folders and we just have an egg-link in lib folder that points to src directory where codejail folder resides and thus it somehow doesn't complaint about the script missing.

Logs from Staging server:
2022-10-27 11:37:27,497 ERROR 7987 [edx.courseware] [user 5216437] [ip 13.244.177.10] capa_module.py:945 - LcpFatalError Encountered for block-v1:edx+Codejail101+2022_t1+type@problem+block@17932f95ce414595822b238d08182c11 Traceback (most recent call last): File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 931, in _extract_context safe_exec( File "/usr/lib/python3.8/contextlib.py", line 75, in inner return func(*args, **kwds) File "/edx/app/edxapp/edx-platform/xmodule/capa/safe_exec/safe_exec.py", line 170, in safe_exec exec_fn( File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/safe_exec.py", line 152, in safe_exec res = jail_code.jail_code( File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/jail_code.py", line 317, in jail_code status, stdout, stderr = run_subprocess_fn( File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 90, in run_subprocess_through_proxy six.reraise(*last_exception) File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/six.py", line 719, in reraise raise value File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 75, in run_subprocess_through_proxy status, stdout, stderr, log_calls = deserialize_out(proxy_stdout.rstrip()) File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 45, in deserialize_out return deserialize_in(bstr.decode('utf8')) File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/codejail/proxy.py", line 40, in deserialize_in return ast.literal_eval(ustr) File "/usr/lib/python3.8/ast.py", line 59, in literal_eval node_or_string = parse(node_or_string, mode='eval') File "/usr/lib/python3.8/ast.py", line 47, in parse return compile(source, filename, mode, flags, File "<unknown>", line 1 /edx/app/edxapp/venvs/edxapp/bin/python: can't open file '/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/proxy_main.py': [Errno 2] No such file or directory ^ SyntaxError: invalid syntax During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 788, in lcp lcp = self.new_lcp(self.get_state_for_lcp()) File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 846, in new_lcp return LoncapaProblem( File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 209, in __init__ self.context = self._extract_context(self.tree) File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 947, in _extract_context raise responsetypes.LoncapaProblemError(msg) xmodule.capa.responsetypes.LoncapaProblemError: Error while executing script code: invalid syntax (<unknown>, line 1) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 359, in student_view self.lcp File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/functional.py", line 48, in __get__ res = instance.__dict__[self.name] = self.func(instance) File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 792, in lcp raise LoncapaProblemError(msg).with_traceback(sys.exc_info()[2]) File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 788, in lcp lcp = self.new_lcp(self.get_state_for_lcp()) File "/edx/app/edxapp/edx-platform/xmodule/capa_module.py", line 846, in new_lcp return LoncapaProblem( File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 209, in __init__ self.context = self._extract_context(self.tree) File "/edx/app/edxapp/edx-platform/xmodule/capa/capa_problem.py", line 947, in _extract_context raise responsetypes.LoncapaProblemError(msg) xmodule.capa.responsetypes.LoncapaProblemError: cannot create LoncapaProblem block-v1:edx+Codejail101+2022_t1+type@problem+block@17932f95ce414595822b238d08182c11: Error while executing script code: invalid syntax (<unknown>, line 1)

kdmccormick · 2022-11-09T20:18:32Z

Huh, interesting. I think part of the problem is that proxy_main.py isn't packaged or used the way that pip expects from a standard Python package. I wonder if we could fix it by deleting proxy_main.py (which is just a simple wrapper) and then just using Python's -m option to invoke the wrapped function directly.

More specifically, I wonder if we could change edx-platform's call from:

python -u proxy_main.py ...

to:

python -u -m codejail.proxy.proxy_main ...

nedbat · 2022-11-09T22:35:59Z

I agree the problem is that I used proxy_main.py from the root of the repo rather than installing it properly. It's not quite as simple as changing the command line, because the function takes sys.argv as an argument, but it should be pretty simple.

kdmccormick force-pushed the kdmccormick/fix-distribution branch from 7c3fa6e to 80a74fc Compare October 3, 2022 15:41

kdmccormick mentioned this pull request Oct 3, 2022

build: add missing files to PyPI distribution (v3.3.0) #138

Closed

kdmccormick requested a review from nedbat October 3, 2022 15:47

nedbat suggested changes Oct 4, 2022

View reviewed changes

kdmccormick force-pushed the kdmccormick/fix-distribution branch from 80a74fc to e42316f Compare October 4, 2022 14:39

kdmccormick requested a review from nedbat October 4, 2022 15:04

nedbat approved these changes Oct 5, 2022

View reviewed changes

kdmccormick merged commit 1a63586 into master Oct 5, 2022

kdmccormick deleted the kdmccormick/fix-distribution branch October 5, 2022 16:07

kdmccormick mentioned this pull request Jan 5, 2023

Install all edx-platform dependencies from PyPI, not GitHub openedx-unsupported/wg-developer-experience#153

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

build: add missing files to PyPI distribution (v3.3.0) #140

build: add missing files to PyPI distribution (v3.3.0) #140

Uh oh!

kdmccormick commented Oct 3, 2022

Uh oh!

kdmccormick commented Oct 3, 2022

Uh oh!

nedbat left a comment

Uh oh!

Uh oh!

Uh oh!

nedbat Oct 4, 2022

Uh oh!

kdmccormick Oct 4, 2022

Uh oh!

nedbat Oct 4, 2022

Uh oh!

kdmccormick Oct 4, 2022

Uh oh!

kdmccormick Oct 4, 2022

Uh oh!

UsamaSadiq commented Oct 6, 2022

Uh oh!

kdmccormick commented Oct 7, 2022

Uh oh!

aht007 commented Oct 28, 2022 •

edited

Loading

Uh oh!

kdmccormick commented Nov 9, 2022

Uh oh!

nedbat commented Nov 9, 2022

Uh oh!

Uh oh!

build: add missing files to PyPI distribution (v3.3.0) #140

build: add missing files to PyPI distribution (v3.3.0) #140

Uh oh!

Conversation

kdmccormick commented Oct 3, 2022

Description

Testing

Some Context

Uh oh!

kdmccormick commented Oct 3, 2022

Uh oh!

nedbat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nedbat Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

nedbat Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

UsamaSadiq commented Oct 6, 2022

Uh oh!

kdmccormick commented Oct 7, 2022

Uh oh!

aht007 commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdmccormick commented Nov 9, 2022

Uh oh!

nedbat commented Nov 9, 2022

Uh oh!

Uh oh!

aht007 commented Oct 28, 2022 •

edited

Loading