Ensure LMS_ROOT_URL is defined in i18n and assets settings modules

[wgu-taylor-payne]

When running the management commands related to translations and static assets in tutor/templates/build/openedx/Dockerfile, LMS_ROOT_URL needs to have a valid value, otherwise the build will fail.

Tutor has relied on there being a valid value for LMS_ROOT_URL present in lms/envs/common.py and cms/envs/common.py, but upcoming changes to edx-platform (#37045) will create the need to ensure LMS_ROOT_URL has a valid value.

This PR stemmed from the discussion here in edx-platform #37045.

[wgu-taylor-payne]

@kdmccormick could you give this a look over?

[regisb]

I think there's a misunderstanding. Asset compilation and translation collection is happening at build time. The LMS root url should only be used at runtime. If not, then it means that we will no longer be able to provide Docker images that are usable by everyone. Images pushed to the Docker registry should not include any personal detail of any specific platform. If static assets or translation files include hard-coded references to the LMS root url, then they will no longer work for everyone.

AFAIU we need to revert this change in edx-platform.

[wgu-taylor-payne]

AFAIU we need to revert this change in edx-platform.

edx-platform #37045 has not been merged into master yet. Is that what you are referring to?

Currently, in master, LMS_ROOT_URL is set to 'https://localhost:18000' in lms/envs/common.py and cms/envs/common.py. The mentioned PR lifts that setting into openedx/envs/common.py and assigns it a value of None, which is the production default (i.e. the default in the production.py files).

Correct me if I'm wrong, but tutor.i18n and tutor.assets will inherit the value of LMS_ROOT_URL from the respective lms and cms common.py files. This value has been 'https://localhost:18000'. But now, with the change to None as the value, tutor images build openedx fails because there are places where LMS_ROOT_URL is referenced during the execution of these commands (it seems like it shouldn't need to be referenced, but it is being referenced).

Although, LMS_ROOT_URL is referenced while running these commands, my assumption is that all translation files and static files are not influenced by this value. Wouldn't this be the case if 'https://localhost:18000' has been used as the value for LMS_ROOT_URL up until this point when executing these commands?

So, my take of it all is that the LMS_ROOT_URL setting should have no bearing when running these commands, but currently it does. We either hold off setting LMS_ROOT_URL to None and make it so the translation and static asset commands can execute without it, or we move forward setting LMS_ROOT_URL to None and just make sure that there is a valid value (non-empty str) defined in i18n.py and assets.py (and then tackle removing the dependency on this setting at a later time).

[regisb]

OK I think a "good" fix would be to define LMS_ROOT_URL in tutor's i18n.py and assets.py, but we cannot use a value that differs from one installation to another, for the reasons highlighted above. In particular, having different py setting files, would cause cache busting during build, which would considerably slow down tutor images build openedx.

So I suggest something like:

LMS_ROOT_URL = "https://lms.local"

[kdmccormick]

In particular, having different py setting files, would cause cache busting during build, which would considerably slow down tutor images build openedx.

I hadn't thought of that, good point.

LMS_ROOT_URL = "https://lms.local"

Until we're able to fix the unnecessary dependency on LMS_ROOT_URL upstream, this sounds like a good compromise to me. I suggest leaving a comment to explain that this is just a dummy value with no bearing on the static asset or i18n build steps.

[kdmccormick]

I merged without a changelog entry because I don't think operators/devs will care about this change, but if you disagree Régis let me know and we can create a follow-up PR.