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.

Sign up for GitHub

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

Jump to bottom

[DEPR]: Asset processing in Paver #31895

Closed
Tracked by #21
, #612
...
kdmccormick opened this issue Mar 8, 2023 · 10 comments
Closed
Tracked by #21
, #612
...

[DEPR]: Asset processing in Paver #31895

kdmccormick opened this issue Mar 8, 2023 · 10 comments
Assignees
@kdmccormick
Labels
depr Proposal for deprecation & removal per OEP-21

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Mar 8, 2023 •
edited
Loading

This Deprecation was accepted on May 3, 2023. It concerned only the Paver asset commands.

At this point, in May 2024, it is now simpler to deprecate Paver in its entirety. Please see:
@kdmccormick kdmccormick added the depr Proposal for deprecation & removal per OEP-21 label Mar 8, 2023
@kdmccormick kdmccormick self-assigned this Mar 8, 2023
This was referenced Mar 10, 2023
Closed
Closed
@kdmccormick
Copy link
Member Author

Announcement: https://discuss.openedx.org/t/deprecation-removal-edx-platform-asset-processing-in-paver/9552

@kdmccormick kdmccormick moved this from Proposed to Communicated in DEPR: Deprecation & Removal Mar 10, 2023
This was referenced Mar 10, 2023
Closed
Merged
kdmccormick added a commit that referenced this issue Mar 16, 2023
@kdmccormick
docs: tweak webpack config design in assets ADR (#31949)
463b64d 
Follow-up from:
#31790 (comment)

Part of DEPR:
#31895
@kdmccormick
Copy link
Member Author

The new implementation won't be landing in Palm, most likely. I pushed the acceptance date out to May 3rd, and pushed the final removal out to Redwood.

@kdmccormick kdmccormick mentioned this issue May 22, 2023
Closed
@kdmccormick kdmccormick moved this from Communicated to Accepted in DEPR: Deprecation & Removal May 30, 2023
@kdmccormick
Copy link
Member Author

This deprecation has been Accepted. Development of the replacement asset script is ongoing (see issues linked in description). When the replacement is ready, I'll put a warning in the output of the old asset processing script and move this issue to Deprecated.

@kdmccormick kdmccormick moved this from New to In Progress in Developer Experience Working Group Jun 14, 2023
@kdmccormick kdmccormick mentioned this issue Jul 10, 2023
Merged
@kdmccormick kdmccormick mentioned this issue Jul 20, 2023
Closed
kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Jul 27, 2023
@kdmccormick
build: commit builtinblocks Webpack config and stub out xmodule_assets
7eb4836 
The Webpack configuration file for built-in XBlock JS used to be
generated at build time and git-ignored. It lived at
common/static/xmodule/webpack.xmodule.config.js. It was generated
because the JS that it referred to was also generated at build-time, and
the filenames of those JS modules were not static.

Now that its contents have been made entirely static [1], there is no
reason we need to continue generating this Webpack configuration file.
So, we check it into edx-platform under the name
./webpack.builtinblocks.config.js. We choose to put it in the repo's
root directory because the paths contained in the config file are
relative to the repo's root.

This allows us to behead both the xmodule/static_content.py
(`xmodule_assets`) script andthe  `process_xmodule_assets` paver task, a
major step in removing the need for Python in the edx-platform asset
build [2]. It also allows us to delete the `HTMLSnippet` class and all
associated attributes, which were exclusively used by
xmodule/static_content.py..

We leave `xmodule_assets` and  `process_xmodule_assets` in as stubs for
now in order to avoid breaking external code (like Tutor) which calls
Paver; the entire pavelib/assets.py function will be eventually removed
soon anyway [3]. Further, to avoid extraneous refactoring, we keep one
method of `HTMLSnippet` around on a few of its former subclasses:
`get_html`. This method was originally part of the XModule framework;
now, it is left over on a few classes as a simple internal helper
method.

References:
1. openedx#32480
2. openedx#31800
3. openedx#31895

Part of: openedx#32481
kdmccormick added a commit that referenced this issue Jul 27, 2023
@kdmccormick
build: commit builtinblocks Webpack config and stub out `xmodule_asse…
3557799 
…ts` (#32685)

The Webpack configuration file for built-in XBlock JS used to be
generated at build time and git-ignored. It lived at
common/static/xmodule/webpack.xmodule.config.js. It was generated
because the JS that it referred to was also generated at build-time, and
the filenames of those JS modules were not static.

Now that its contents have been made entirely static [1], there is no
reason we need to continue generating this Webpack configuration file.
So, we check it into edx-platform under the name
./webpack.builtinblocks.config.js. We choose to put it in the repo's
root directory because the paths contained in the config file are
relative to the repo's root.

This allows us to behead both the xmodule/static_content.py
(`xmodule_assets`) script andthe  `process_xmodule_assets` paver task, a
major step in removing the need for Python in the edx-platform asset
build [2]. It also allows us to delete the `HTMLSnippet` class and all
associated attributes, which were exclusively used by
xmodule/static_content.py..

We leave `xmodule_assets` and  `process_xmodule_assets` in as stubs for
now in order to avoid breaking external code (like Tutor) which calls
Paver; the entire pavelib/assets.py function will be eventually removed
soon anyway [3]. Further, to avoid extraneous refactoring, we keep one
method of `HTMLSnippet` around on a few of its former subclasses:
`get_html`. This method was originally part of the XModule framework;
now, it is left over on a few classes as a simple internal helper
method.

References:
1. #32480
2. #31800
3. #31895

Part of: #32481
@kdmccormick
Copy link
Member Author

The new asset processing suite has been merged as "experimental". I will continue to test it, and will communicate when it's time to mark the Paver assets suite as deprecated and encourage folks to switch to the new suite.

@kdmccormick kdmccormick mentioned this issue Sep 14, 2023
Closed
kdmccormick added a commit to kdmccormick/tutor that referenced this issue Oct 17, 2023
@kdmccormick
build: switch to new edx-platform asset build
ff320c6 
TODO:
* Test dev & prod builds one last time
* Test watching again
* Print warning in openedx-assets
* Add REMOVE-AFTER-VXX comments
* Update docs

Part of: openedx/edx-platform#31895
@kdmccormick
Copy link
Member Author

I am hoping to mark the old asset build as deprecated in Quince so we can remove it in Redwood, although the content libraries V2 project is competing for my time. Here's my WIP PR: kdmccormick/tutor#37

kdmccormick added a commit to kdmccormick/tutor that referenced this issue Apr 18, 2024
@kdmccormick
build: switch to new edx-platform asset build
7e9a35e 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry, or the Open edX DEPR ticket:
openedx/edx-platform#31895
kdmccormick added a commit to kdmccormick/tutor that referenced this issue Apr 18, 2024
@kdmccormick
build: switch to new edx-platform asset build
b062098 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry, or the Open edX DEPR ticket:
openedx/edx-platform#31895
kdmccormick added a commit to kdmccormick/tutor that referenced this issue Apr 18, 2024
@kdmccormick
feat!: remove dependency on Paver scripts
344f9a7 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry.
For further details and rationale, see the upstream DEPR ticket:
openedx/edx-platform#31895
@kdmccormick kdmccormick mentioned this issue Apr 18, 2024
Merged
@kdmccormick kdmccormick moved this from Accepted to Deprecated in DEPR: Deprecation & Removal Apr 19, 2024
KyryloKireiev pushed a commit to raccoongang/edx-platform that referenced this issue Apr 24, 2024
@kdmccormick @KyryloKireiev
build: compile/watch sass with new npm scripts
ac6c929 
`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
@robrap robrap removed this from Arch-BOM Apr 29, 2024
@robrap robrap mentioned this issue Apr 29, 2024
Closed
7 tasks
@kdmccormick
Copy link
Member Author

I am rolling this DEPR into a larger proposed deprecation of all edx-platform Paver usage: #34467

@kdmccormick kdmccormick closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Maintenance Apr 30, 2024
@github-project-automation github-project-automation bot moved this from Deprecated to Removed in DEPR: Deprecation & Removal Apr 30, 2024
@github-project-automation github-project-automation bot moved this to Done in Arch-BOM Apr 30, 2024
kdmccormick added a commit to kdmccormick/tutor that referenced this issue May 6, 2024
@kdmccormick
feat!: remove dependency on Paver scripts
554176e 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry.
For further details and rationale, see the upstream DEPR ticket:
openedx/edx-platform#31895
kdmccormick added a commit to kdmccormick/tutor that referenced this issue May 7, 2024
@kdmccormick
feat!: remove dependency on Paver scripts
610135c 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry.
For further details and rationale, see the upstream DEPR ticket:
openedx/edx-platform#31895
kdmccormick added a commit to kdmccormick/tutor that referenced this issue May 14, 2024
@kdmccormick
feat!: remove dependency on Paver scripts
f00c311 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry.
For further details and rationale, see the upstream DEPR ticket:
openedx/edx-platform#31895
kdmccormick added a commit to overhangio/tutor that referenced this issue May 16, 2024
@kdmccormick
feat!: remove dependency on Paver scripts (#1042)
c84a741 
BREAKING CHANGE: `openedx-assets` is replaed with `npm run` subcommands.
For details, see the changelog entry.
For further details and rationale, see the upstream DEPR ticket:
openedx/edx-platform#31895
@robrap robrap mentioned this issue Jun 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kdmccormick kdmccormick

Labels
depr Proposal for deprecation & removal per OEP-21
Projects
DEPR: Deprecation & Removal
Archived in project
Maintenance
Archived in project
Developer Experience Working Group
Status: In Progress
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kdmccormick @robrap