WIP: Add back some auto-configuration so we can use from edx-platform. #51

Open
wants to merge 1 commit into
from

Projects

None yet

2 participants

@nedbat
Member
nedbat commented Jan 9, 2017

No description provided.

@nedbat
Member
nedbat commented Jan 9, 2017

I don't like that there are three places that say "bin/python". Some of this could go away if we drop the CODEJAIL_TEST_VENV environment variable. It isn't documented in the readme, and I managed the same effect by activating the edx-platform venv before running the codejail tests.

@nedbat nedbat changed the title from Add back some auto-configuration so we can use from edx-platform. to WIP: Add back some auto-configuration so we can use from edx-platform. Jan 9, 2017
@nedbat
Member
nedbat commented Jan 9, 2017

I'm not done with this, but it'd be good to get some eyeballs on it early :)

codejail/safe_exec.py
+ venv = sibling_sandbox_venv()
+ if not venv:
+ raise Exception("Couldn't find sandbox virtualenv")
+ configure("python", os.path.join(venv, "bin/python"), user="sandbox", lang=languages.python2)
@jcdyer
jcdyer Jan 9, 2017 Member

I don't think this is the right place to add this logic. edx-platform should be deciding where it wants its sandboxes created, not codejail.

@jcdyer
jcdyer Jan 9, 2017 Member

At the most, codejail should provide an option to autocreate a sibling env, but the importing code should have to opt in to it, perhaps by an environment variable, or some other setting.

@nedbat
nedbat Jan 9, 2017 Member

I would love for you to help figure out how to do that. Keep in mind that the tests need pass, both with and with sandboxes in the environment. We have Django middleware that configures codejail, but that middleware isn't run during the safe_exec tests, since they are not in a Django app.

@@ -101,16 +95,26 @@ def not_safe_exec(
globals_dict.update(json_safe(g_dict))
-if ALWAYS_BE_UNSAFE: # pragma: no cover
- # Make safe_exec actually call not_safe_exec, but log that we're doing so.
+def configure_python():
@jcdyer
jcdyer Jan 9, 2017 Member

Thanks! Let me know when this is ready for a for-real review.

@nedbat nedbat Updates to use the latest refactoring with edx-platform
* Python 2.x is called "python" again

* Provide "configure_python" so that edx-platform can get convenient
  configuration.
9d48ad3
@nedbat
Member
nedbat commented Jan 10, 2017

@jcdyer ready for real review.

@jcdyer
jcdyer approved these changes Jan 10, 2017 View changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment