Stop using global state to store configuration options
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 479
- Forks
- 84
- Avg merge
- 16h 34m
- Merged PRs (30d)
- 2
Description
Codejail has a global variable at codejail.jail_code.COMMANDS. It's a dictionary that maps command names (like "python") to safe shell commands. An example value is:
{'python': {'cmdline_start': ['/edx/app/edxapp/venvs/edxapp-sandbox/bin/python', '-E', '-B'], 'user': 'sandbox'}}
The dictionary is populated by calling codejail.jail_code.configure, which is generally done via the codejail.django_integration middleware.
This all works fine, but the fact that COMMANDS is a global variable is not idiomatic Python. More concretely, it's a pain in the neck for unit tests: the order and timing with which unit tests are run will affect whether .configure has been called or not. This could also cause problems across Python processes in production, leading to situations where codejail is configured in one process but effectively disabled in another process without anyone knowing. Not good.
This stateful system also forces us to use a middleware in order to configure codejail in Django--that's another layer where something could go wrong.
I recommend replacing COMMANDS with either:
- a stateless function named
load_commands(), which is safe to call over and over again, which does something like this:try: from django.conf import settings except ImportError, AttributeError settings = {} if settings.get('CODE_JAIL'): return _load_command_from_django_settings(settings) else: # .... load alternative/fallback configuration options, if we're not using Django - putting the entire Codejail interface behind a class, so clients would need to pass the settings into it, like this:
from django.conf import settings codejail = CodeJail(some_setting=settings.CODEJAIL[...], etc..) codejail.safe_exec(...)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading codejail.jail_code.COMMANDS and configure, then inspect the codejail.django_integration middleware that populates the configuration. Decide on and implement a non-global configuration design, with repeated configuration independent of unit-test order and behavior consistent across Python processes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- django, python
- Domain
- backend, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100