build: compile/watch sass with new npm scripts #34318

kdmccormick · 2024-03-01T16:37:12Z

Description

paver commands are deprecated for managing static assets. Starting in Sumac, only npm run commands will be supported for managing static assets.

To ease the transition, both paver and npm run commands will work in Redwood. However, we want to stop using the implementations of the paver asset commands right now, as they are blocking the Python 3.11 upgrade. This will also make the removal of paver commands more straightforward come Sumac.

So, this commit turns these commands/functions:

paver compile_sass (used by configuration)
paver watch_sass (used by configuration and devstack)
pavelib/assets.py:_compile_sass (used by Tutor)

into very thin wrappers around the new npm run commands. Each of these paver routines now raise a loud deprecation warning, including a message of the npm run command that the operator can switch to.

We expect no impact to site operators or end users.

Also included in separate commits

fix: simplify npm run watch-sass so that it is more reliable
docs: npm scripts for static assets are no longer experimental :)

Supporting Information

[DEPR]: Asset processing in Paver #31895

Testing

With Tutor

Check out this branch
Install a theme, such as indigo: tutor plugins install indigo
Mount your local edx-platform repo: tutor mounts add <path to your edx-platform>
tutor dev stop && tutor local stop
tutor config save
tutor images build openedx openedx-dev
Test results of prod Sass build
- tutor local start -d
- tutor local do settheme indigo
- Browse some Django-rendered pages. Good examples are the Studio outline editor, LMS Course Preview (accessible from any edited Studio unit by hitting "Preview"), the Instructor Dashboard, and the LMS XBlock Views within any course.
- tutor local do settheme default
- Browse some Django-rendered pages
Test results of dev Sass build
- tutor dev start -d
- Browse some Django-rendered pages
- tutor dev do settheme indigo
- Browse some Django-rendered pages
Test hot Sass recompilation
- tutor dev run lms env EDX_PLATFORM_THEME_DIRS=/openedx/themes npm run watch-sass
- touch "$(tutor config printroot)/env/build/openedx/themes/indigo/lms/static/sass/_background.scss"
- Confirm that the theme rebuilds
- NOTE: The Tutor-recommend command tutor dev run watchthemes seems to lead to an infinite recompilation loop any time any themed Sass file is changed. I have authored this PR to be technically compatible with tutor dev run watchthemes, but I'm not worried about fixing the infinite recompilation bug here.

Without Tutor

To test outside of Tutor, I recommend just building static assets however you normally would. For example, I believe paver update_assets will do the trick for folks using devstack and configuration.

Merge Considerations

I am aiming for this week (Thu March 28 if possible) in order to unblock the Python upgrade, which needs to be complete for the Redwood cut in late April.

Priorities after merging this:

build: remove dependency on Python sass module #34439 (This will unblock the Python 3.11 upgrade)
Update ./manage.py compile_sass, which is used for site theme compilation, to use npm run directly instead of going through paver
feat!: switch to new edx-platform asset build kdmccormick/tutor#37
Use the new edx-platform assets build + other improvements kdmccormick/tutor#34

open-craft-grove · 2024-03-21T18:09:55Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-22T08:23:42Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-22T09:13:33Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-25T14:03:17Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-25T14:35:23Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-26T19:06:09Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-26T21:36:25Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-26T22:36:55Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-27T13:53:04Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-04-01T18:05:20Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-04-03T14:51:32Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

kdmccormick · 2024-04-03T17:10:25Z

@feanil friendly reminder about this PR.

The follow-up PR is also ready for review, if you want to review them in one swoop:

build: remove dependency on Python sass module #34439

open-craft-grove · 2024-04-03T17:53:15Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

feanil

Looks good, I was able to run the assets commands and see things working locally.

The implementation of `npm run watch-sass` was trying really hard to recompile precisely only the Sass that needed to be recompiled, but in order to do so, it had to spin up several `watchmedo` processes per theme. These processes would trigger one another sometimes, leading to infinite recompilation loops. Rather than figure out all the dependency directions and messing with `watchmedo`, I've opted to simplify the script to invoke a single `watchmedo` process per theme. A single theme recompiles within seconds, so I think this is a good compromise, one which makes the script easier to reason about will help me move pass this legacy assets work.

`paver` commands are deprecated for managing static assets. Starting in Sumac, only `npm run` commands will be supported for managing static assets. To ease the transition, both `paver` and `npm run` commands will work in Redwood. However, we want to stop using the *implementations* of the `paver` asset commands right now, as they are blocking the Python 3.11 upgrade. This will also make the removal of `paver` commands more straightforward come Sumac. So, this commit turns these commands/functions: * paver compile_sass (used by configuration) * paver watch_sass (used by configuration and devstack) * pavelib/assets.py:_compile_sass (used by Tutor) into very thin wrappers around the new `npm run` commands. Each of these paver routines now raise a loud deprecation warning, including a message of the `npm run` command that the operator can switch to. We expect no impact to site operators or end users. openedx#31895

edx-pipeline-bot · 2024-04-04T15:50:17Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2024-04-04T16:07:16Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2024-04-04T16:07:17Z

2U Release Notice: This PR has been deployed to the edX production environment.

kdmccormick self-assigned this Mar 1, 2024

kdmccormick force-pushed the kdmccormick/libsass-python branch from 774415c to f233e8d Compare March 18, 2024 17:12

kdmccormick changed the title ~~build: turn more Paver code into a dumb wrapper around NPM build scripts~~ build: switch to new Sass build, with compatibility wrapper for paver Mar 20, 2024

kdmccormick changed the title ~~build: switch to new Sass build, with compatibility wrapper for paver~~ build: switch to new Sass build, with temporary paver compat wrapper Mar 20, 2024

kdmccormick changed the title ~~build: switch to new Sass build, with temporary paver compat wrapper~~ build: switch to npm compile-sass, with temporary paver compat wrapper Mar 20, 2024

kdmccormick changed the title ~~build: switch to npm compile-sass, with temporary paver compat wrapper~~ build: make paver compile_sass wrap npm run compile-sass Mar 20, 2024

kdmccormick mentioned this pull request Mar 20, 2024

[DEPR]: Asset processing in Paver #31895

Closed

kdmccormick force-pushed the kdmccormick/libsass-python branch 2 times, most recently from c9048ea to 936b18a Compare March 21, 2024 17:34

kdmccormick added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Mar 21, 2024

kdmccormick force-pushed the kdmccormick/libsass-python branch from 936b18a to 2b3d56b Compare March 25, 2024 12:52

kdmccormick force-pushed the kdmccormick/libsass-python branch from 2b3d56b to c174d83 Compare March 26, 2024 18:29

kdmccormick force-pushed the kdmccormick/libsass-python branch from c174d83 to 42fe781 Compare March 26, 2024 20:45

kdmccormick changed the title ~~build: make paver compile_sass wrap npm run compile-sass~~ build: switch to npm run compile-sass Mar 26, 2024

kdmccormick changed the title ~~build: switch to npm run compile-sass~~ build: compile/watch sass with using new npm scripts Mar 26, 2024

kdmccormick changed the title ~~build: compile/watch sass with using new npm scripts~~ build: compile/watch sass with new npm scripts Mar 26, 2024

kdmccormick force-pushed the kdmccormick/libsass-python branch from 42fe781 to b6a6ffb Compare March 26, 2024 21:53

kdmccormick marked this pull request as ready for review March 26, 2024 21:55

kdmccormick force-pushed the kdmccormick/libsass-python branch from b6a6ffb to 0c200ea Compare March 27, 2024 13:13

kdmccormick mentioned this pull request Mar 28, 2024

build: remove dependency on Python sass module #34439

Merged

kdmccormick force-pushed the kdmccormick/libsass-python branch from 0c200ea to 9d027dc Compare April 1, 2024 17:24

kdmccormick force-pushed the kdmccormick/libsass-python branch from 9d027dc to a643b8c Compare April 3, 2024 14:05

kdmccormick force-pushed the kdmccormick/libsass-python branch from a643b8c to 49a2f38 Compare April 3, 2024 17:07

feanil approved these changes Apr 3, 2024

View reviewed changes

kdmccormick added 3 commits April 4, 2024 10:01

docs: npm scripts for static assets are no longer experimental :)

e33302b

kdmccormick force-pushed the kdmccormick/libsass-python branch from 49a2f38 to a228b81 Compare April 4, 2024 14:01

kdmccormick merged commit 24db4df into openedx:master Apr 4, 2024
67 checks passed

kdmccormick deleted the kdmccormick/libsass-python branch April 4, 2024 14:31

kdmccormick mentioned this pull request Apr 4, 2024

revert: build: compile/watch sass with new npm scripts #34473

Closed

kdmccormick mentioned this pull request Apr 18, 2024

feat!: remove dependency on Paver scripts overhangio/tutor#1042

Merged

kdmccormick mentioned this pull request May 1, 2024

[DEPR]: Paver #34467

Open

kdmccormick mentioned this pull request Jun 21, 2024

Possible memory leak in watchtheme overhangio/tutor#1082

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: compile/watch sass with new npm scripts #34318

build: compile/watch sass with new npm scripts #34318

kdmccormick commented Mar 1, 2024 •

edited

Loading

open-craft-grove commented Mar 21, 2024

open-craft-grove commented Mar 22, 2024

open-craft-grove commented Mar 22, 2024

open-craft-grove commented Mar 25, 2024

open-craft-grove commented Mar 25, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 27, 2024

open-craft-grove commented Apr 1, 2024

open-craft-grove commented Apr 3, 2024

kdmccormick commented Apr 3, 2024

open-craft-grove commented Apr 3, 2024

feanil left a comment

edx-pipeline-bot commented Apr 4, 2024

edx-pipeline-bot commented Apr 4, 2024

edx-pipeline-bot commented Apr 4, 2024

build: compile/watch sass with new npm scripts #34318

build: compile/watch sass with new npm scripts #34318

Conversation

kdmccormick commented Mar 1, 2024 • edited Loading

Description

Also included in separate commits

Supporting Information

Testing

With Tutor

Without Tutor

Merge Considerations

open-craft-grove commented Mar 21, 2024

open-craft-grove commented Mar 22, 2024

open-craft-grove commented Mar 22, 2024

open-craft-grove commented Mar 25, 2024

open-craft-grove commented Mar 25, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 26, 2024

open-craft-grove commented Mar 27, 2024

open-craft-grove commented Apr 1, 2024

open-craft-grove commented Apr 3, 2024

kdmccormick commented Apr 3, 2024

open-craft-grove commented Apr 3, 2024

feanil left a comment

Choose a reason for hiding this comment

edx-pipeline-bot commented Apr 4, 2024

edx-pipeline-bot commented Apr 4, 2024

edx-pipeline-bot commented Apr 4, 2024

kdmccormick commented Mar 1, 2024 •

edited

Loading