Add PYTHONBREAKPOINT as exposed config variable

[Carlos-Muniz]

Criteria

• Exposes PYTHONBREAKPOINT as a Tutor configuration variable
• Sets ENV PYTHONBREAKPOINT={{ PYTHONBREAKPOINT }} in the openedx dockerfile
• Changes the docs to recommend using breakpoint(). Explains that PYTHONBREAKPOINT can be modified in order to use a custom debugger.

Testing

1. Build openedx: tutor images build openedx
2. Build openedx-dev: tutor dev dc build
3. Run bash in lms: tutor dev run lms bash
4. Within bash, check for PYTHONBREAKPOINT: env | grep PYTHONBREAKPOINT

And make test

Closes openedx-unsupported/wg-developer-experience#45

[regisb]

Can you please add an entry to the CHANGELOG.md file? https://docs.tutor.overhang.io/tutor.html#contributing Also, please squash your commits.

[kdmccormick]

@regisb I am a little confused by your feedback. The goal of openedx-unsupported/wg-developer-experience#45 was to default to ipdb.set_trace (as ipdb is already installed into the image) while allowing users to easily configure a different debugger. Why would we install ipdb if pdb is going to be the default? And why is a plugin better than a config setting here?

Now, I do realize that a Docker build ARG might not be the best way to do this, since it requires a rebuild -- perhaps setting PYTHONBREAKPOINT in the environment section of env/dev/docker-compose.yml would be better.

[regisb]

I understand your confusion @kdmccormick, as there was a one-letter typo in my previous comment...

Let me recap:

1. We should have ipdb.set_trace() as the default breakpoint in the openedx development Docker image.
2. There should not be a Tutor configuration setting to govern that value.
3. There is no need to add a build-time Docker ARG.
4. Users who want to override the default breakpoint should override the runtime PYTHONBREAKPOINT environment variable.

[kdmccormick]

@regisb

We should have ipdb.set_trace() as the default breakpoint in the openedx development Docker image.

👍🏻

There should not be a Tutor configuration setting to govern that value.

I'll respect your discretion here but it is not clear to me where the line is between "expose a config setting" and "just let users override it with a plugin". Do you have a general philosophy that @Carlos-Muniz and I can use in the future?

There is no need to add a build-time Docker ARG.

Okay, so I imagine the Dockerfile line would just be:

ENV PYTHONBREAKPOINT=ipdb.set_trace

(@Carlos-Muniz ^)

Users who want to override the default breakpoint should override the runtime PYTHONBREAKPOINT environment variable.

How would a user do this (other than manually setting PYTHONBREAKPOINT=... every time they enter an LMS/CMS bash shell)? Is there patch or hook I am missing for runtime environment variables?

[regisb]

I'll respect your discretion here but it is not clear to me where the line is between "expose a config setting" and "just let users override it with a plugin". Do you have a general philosophy that @Carlos-Muniz and I can use in the future?

I've had this conversation multiple times in a few recent issues; here are a few pointers:

#675 (comment)
#677 (comment)

Basically, the policy is "do not introduce a new setting for a small percentage of users".

I realize that I need to better articulate this policy and make it clearer to contributors. This will be resolved in #683.

How would a user do this (other than manually setting PYTHONBREAKPOINT=... every time they enter an LMS/CMS bash shell)? Is there patch or hook I am missing for runtime environment variables?

Either the "openedx-dev-dockerfile-post-python-requirements" or "openedx-dockerfile-final" patch can be used to override this environment variable in a more sustainable way.

[Carlos-Muniz]

@regisb I've addressed your comments and can squash if there are no further concerns.

[regisb]

Thanks for this PR Carlos! and sorry about the back-and-forth comments...