-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
build: finish replacing paver assets #34554
build: finish replacing paver assets #34554
Conversation
1b9ca1c
to
289e75e
Compare
ecaa339
to
d936472
Compare
8d15a9d
to
7757d59
Compare
README.rst
Outdated
|
||
Provision your database:: | ||
|
||
# (TODO: we need a step here to create the database itself) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# (TODO: we need a step here to create the database itself) | |
# (TODO: we need a step here to create the database itself) |
Gotta look into what Tutor does and update this. @feanil , if you have the command you ran written down, let me know and I'll drop it in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been running a minimal docker compose
with the following config:
services:
mysql:
image: mysql:8.0
environment:
MYSQL_DATABASE: "edxapp"
MYSQL_USER: "edxapp001"
MYSQL_PASSWORD: "password"
MYSQL_RANDOM_ROOT_PASSWORD: true
ports:
- 3306:3306
mongo:
image: mongo:7
environment:
MONGO_INITDB_DATABASE: 'edxapp'
ports:
- 27017:27017
volumes:
- type: bind
source: mongo-entrypoints.d
target: /docker-entrypoint-initdb.d/
read_only: true
memcache:
image: memcached
ports:
- 11211:11211
with a mongo entrypoint file in the folder with the following content:
db.createUser(
{
user: "edxapp",
pwd: "password", // cleartext dev password, change if you're gonna use this for production.
roles: [
{ role: "readWrite", db: "edxapp" },
]
}
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments but generally looks good.
README.rst
Outdated
|
||
Provision your database:: | ||
|
||
# (TODO: we need a step here to create the database itself) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been running a minimal docker compose
with the following config:
services:
mysql:
image: mysql:8.0
environment:
MYSQL_DATABASE: "edxapp"
MYSQL_USER: "edxapp001"
MYSQL_PASSWORD: "password"
MYSQL_RANDOM_ROOT_PASSWORD: true
ports:
- 3306:3306
mongo:
image: mongo:7
environment:
MONGO_INITDB_DATABASE: 'edxapp'
ports:
- 27017:27017
volumes:
- type: bind
source: mongo-entrypoints.d
target: /docker-entrypoint-initdb.d/
read_only: true
memcache:
image: memcached
ports:
- 11211:11211
with a mongo entrypoint file in the folder with the following content:
db.createUser(
{
user: "edxapp",
pwd: "password", // cleartext dev password, change if you're gonna use this for production.
roles: [
{ role: "readWrite", db: "edxapp" },
]
}
)
22c0a7c
to
debfec8
Compare
Thanks @feanil . I decided to punt on this:
I think it'd be good, when we have some more time, to document the "bare metal" steps in a more thoughtful way. Perhaps a reference or how-to page, plus check-ins of your docker-compose files, and maybe even add something in CI test it. For now, rather than adding commands or files that I haven't tested, I just added general text that I hope could point more advanced operators in the right direction:
|
debfec8
to
94fde61
Compare
94fde61
to
a90edce
Compare
Together, these changes make it so that all features of the Paver-based asset compilation system are supported with drop-in Paver-free replacements. The remaining Paver asset functions are trivial wrappers, which can be comfortably deleted before Sumac * Turn `./manage.py ... compile_sass` into a simple wrapper around `npm run compile-sass` * Turn `paver webpack` into a simple wrapper around `npm run webpack` * Turn `pavelib.assets:collect_assets` into a simple wrapper around `./manage.py ... collectstatic` * Add/improve deprecation warnings for all Paver asset commands. * Load defaults for asset-related Django settings from environment variables. This allows the build to work without Python. For the settings which will be removed in Sumac, I've added deprecation warnings. * Change EDX_PLATFORM_THEME_DIRS env var to COMPREHENSIVE_THEME_DIRS. This simplifies the migration instructions, because all the new env vars now match their corresponding Django settings. This amends an ADR, but it should not be a breaking change because the env var was recently added (since Quince) and nobody should be using it yet. * Future-proof the static assets ADR with links. The linked pages will be kept up-to-date even if the ADR isn't. Part of: openedx#34467
a90edce
to
339d95d
Compare
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
This reverts commit 3f0f7ce.
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
Reverts #34554, which causes compilation of edX.org's legacy comprehensive theme to be skipped in their deployment pipeline. We have not determined the precise cause yet, but it seems like the compile_sass management command is not correctly getting the list of comprehensive theme directories from Django settings.
Description (for squashed commit)
Together, these changes make it so that all features of the Paver-based asset compilation system are supported with drop-in Paver-free replacements. The remaining Paver asset functions are trivial wrappers, which can be comfortably deleted before Sumac.
./manage.py ... compile_sass
into a simple wrapper aroundnpm run compile-sass
paver webpack
into a simple wrapper aroundnpm run webpack
pavelib.assets:collect_assets
into a simple wrapper around./manage.py ... collectstatic
Add a static asset guide at docs/static-assets.rst, and improve the "bare metal" docs in the READMEextracted and merged here: docs: improve 'bare metal' docs and add static assets reference #34678Part of: #34467
Merge considerations
This needs to merge before Redwood (May 9) so that Paver can be safely removed before Sumac.
As soon as this merges, I'll re-communicate the Paver DEPR, which I've re-written to encompass all Paver commands and Paver-related settings.
Testing
Automated
All changed code will be hit by the static assets check and CI. I've also updated the paver and compile_sass test suites to check that the wrapped npm commands are as expected.
Manual
In a fresh edx-platform clone, I checked out master. Set up a venv and ran
paver update_assets
.In another fresh clone, I checked out this branch. Set up venv and ran
paver update_assets
.I compared the contents of the two clones, ensuring that built assets were identical.
Finally, in my Tutor environment, I ran
make clean
to delete all built assets, and then rebuilt withCOMPREHENSIVE_THEMES_DIR=/openedx/themes paver update_assets
. I then smoke-tested the LMS/CMS frontends.