Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paver deprecation - attempt #? #138

Merged

Conversation

dianakhuang
Copy link
Member

@dianakhuang dianakhuang commented Dec 17, 2024

Once more into the breach.

This attempt removes the extraneous npm install call.

More importantly, it adds an extra call to load django before running webpack. This forces the workers.json file to get written out: https://github.com/openedx/edx-proctoring/blob/73c7f55e2be91324fa07fec6e6ac0a667fdd8412/edx_proctoring/apps.py#L46 This file is then important to ensure that the webpack-workers plugin is then initialized and run: https://github.com/openedx/edx-platform/blob/master/webpack.common.config.js#L57

The JS_ENV_EXTRA_CONFIG is also required to be set even though it is set to an empty dict.

Reverts #137

@dianakhuang dianakhuang force-pushed the revert-137-revert-133-revert-132-diana/revert-paver-changes branch 3 times, most recently from b59eca8 to f861c19 Compare December 17, 2024 23:40
In the specific case of webworkers for proctoring, we need to
force Django to load before we run `npm run webpack` so that
the file ../workers.json gets written. See
https://github.com/openedx/edx-proctoring/blob/73c7f55e2be91324fa07fec6e6ac0a667fdd8412/edx_proctoring/apps.py#L46

Also adds JS_ENV_EXTRA_CONFIG, which is required
to create the webworker config.
@dianakhuang dianakhuang force-pushed the revert-137-revert-133-revert-132-diana/revert-paver-changes branch from f861c19 to a1a1b23 Compare December 17, 2024 23:56
@timmc-edx timmc-edx merged commit a0f7515 into master Dec 18, 2024
3 checks passed
@timmc-edx timmc-edx deleted the revert-137-revert-133-revert-132-diana/revert-paver-changes branch December 18, 2024 14:31
{% if edxapp_staticfiles_storage_overrides %}
{% for override in edxapp_staticfiles_storage_overrides %}
export STATICFILES_STORAGE={{ override | quote }}
sudo -E -H -u {{ edxapp_user }} \
env "PATH=$PATH" \
npm run webpack \
{{ edxapp_venv_bin }} python manage.py lms --settings=$EDX_PLATFORM_SETTINGS print_setting STATIC_ROOT WEBPACK_CONFIG_PATH \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call (or the other one) caused a very unhelpful error during the Ansible run: env: ‘/edx/app/edxapp/venvs/edxapp/bin’: Permission denied

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh, I see the problem...

Suggested change
{{ edxapp_venv_bin }} python manage.py lms --settings=$EDX_PLATFORM_SETTINGS print_setting STATIC_ROOT WEBPACK_CONFIG_PATH \
{{ edxapp_venv_bin }}/python manage.py lms --settings=$EDX_PLATFORM_SETTINGS print_setting STATIC_ROOT WEBPACK_CONFIG_PATH \

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants