-
Notifications
You must be signed in to change notification settings - Fork 421
feat: mount src/ into /edx/src of frontend containers #790
feat: mount src/ into /edx/src of frontend containers #790
Conversation
All micro-frontends in Devstack follow the frontend-app-* naming convention except course-authoring, gradebook, and program-console. This was presumably done to reduce typing, but adds confusion via its inconsistency. Furthermore, the upcoming change (in which backend services will depend on MFEs) will make it so these frontend names will be referenced much less often in the typical dev workflow. For example, `make dev.up.lms` will be sufficient to start Gradebook, so typing `make dev.up.frontend-app-gradebook` soon won't be necessary. This is a breaking interface change in that it renames the commands needed to start/stop/manage these frontends; however, the change is not expected to be breaking in any other way.
Diffing / spotting issues in repetative config can be difficult. Alphabetization makes it easier.
It was once separated out because the idea of DEFAULT_SERVICES and a generalized `dev.up.*` command hadn't been developed, so everything in docker-compose.yml was always started. This is no longer the case (the Registrar extra service exists in docker-compose.yml yet isn't started by default). So, the separation of docker-compose-xqueue.yml is just confusing. Furthermore, by pulling xqueue and xqueue_consumer into docker-compose.yml, we can properly define mysql57 as its dependency.
lms now depends on frontend-app-learning; previously, the converse was true. Running `make dev.up.lms` will start the frontend-app-learning container. This change has been made for all Devstack frontends/backends. This is breaking in that `make dev.up.frontend-app-*` is no longer the best way to start a frontend from a clean state, since it will not start related backend service(s). Instead, the backend service should be started, which will cause all related frontend apps to be started as dependencies. See included ADR (#3) for more context.
Should allow installation of local versions of NPM packages via module.config.js, allowing frontend devs to test out frontend library changes within devstack. The mounts follow the same pattern that micro-services do, which allows devs to develop local versions of Python packages alongside devstack micro-services.
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.
Hm, not sure I understand the syntax there. The first volume on each has the form like:
${DEVSTACK_WORKSPACE}/frontend-app-payment
Whereas the new line is:
${DEVSTACK_WORKSPACE}/src
Doesn't that imply that frontend-app-payment
is a peer of src
, rather than inside src
?
When using module.config.js, the module is a peer of the MFE, i.e.:
frontend-app-payment
paragon
It seems like this code is implying it would be:
frontend-app-payment
src/paragon
Which seems wrong. Am I misunderstanding?
@davidjoy You're understanding is right. See my comment here: https://github.com/edx/devstack/pull/783#discussion_r658843094 In short -- I made this PR to match what backend services do right now. If it's not the right approach for frontends, we can adjust this PR so, say, we are mounting sibling packages from the host into sibling packages in the container (so, I have docker-compose chops but not very many frontend dev chops, so if you have a recommendation on what the best setup is, I can try my hand at making it happen :) |
Heyo - yeah, I think the right thing to do would be to mount the libraries as siblings of the frontend-app, which I think mirrors how everyone has their dev setups organized (I'd guess). I wish there were a way we could avoid explicitly mounting libraries that we expect people may want to use with module.config.js, as I expect the list will keep expanding over time and folks will get confused when new ones don't work. frontend-lib-special-exams for example. Can we mount with a wildcard? Such as all directories that match |
86779ca
to
d7bbc3b
Compare
Closing in favor of https://github.com/edx/devstack/pull/795 |
Reopening this here: https://github.com/edx/devstack/pull/803 |
Should allow installation of local versions of NPM
packages via module.config.js, allowing frontend devs
to test out frontend library changes within devstack.
The mounts follow the same pattern that micro-services
do, which allows devs to develop local versions of
Python packages alongside devstack micro-services.
(this is a PR into another PR)