-
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
feat: add mfe config api #30473
feat: add mfe config api #30473
Conversation
Thanks for the pull request, @MaferMazu! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
@MaferMazu Thank you for your contribution. Please let me know once it is ready. |
Thanks for this, @MaferMazu! It's on my queue to review ASAP. |
Thanks @arbrandes |
f56c223
to
0aebf4b
Compare
6775936
to
6f30f77
Compare
I'm not sure if this is the final location for this application, either way, please consider this ADR for new applications that's underway. |
Thanks a lot for that info @mariajgrimaldi I am going to take a look and make a decision about the location for this app. |
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.
The API looks very lightweight and I love that. I left a note inline, but once that is fixed (or discussed at least) I would approve.
We could remove the draft label though. |
Thanks @mariajgrimaldi for mentioning the ADR for new applications, however, this PR allows configuration in runtime with mfes and we would like to solve this problem in the community, for this reason, I still want to include this little API. I am also super attentive to any comments. |
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
@MaferMazu, congrats on the merge, and thank you for your patience! |
EdX Release Notice: This PR has been deployed to the production environment. |
Thanks for all your work on this! It is going to help us provide a reasonably-sized, pre-built Tutor MFE image, which is something that folks have wanted for a long time. If it's not too late, I have a couple suggested changes:
|
Hey @MaferMazu and @felipemontoya, I've proposed some changes (including the ones I mentioned above) to the API: #30830 Would either of you mind taking a look? |
feat: add mfe config api
feat: add mfe config api (cherry picked from commit 0bb4577)
feat: add mfe config api
feat: add mfe config api
Description
This PR adds MFE Config API. This is part of the work that is being done to obtain the MFE Runtime Configurations and that has been discussed in the BTR WG.
Testing instructions
2.1. In the tutor_root (to know your tutor_root:
tutor config printroot
), in the env > apps > openedx > config > lms.env.json file add"ENABLE_MFE_CONFIG_API": true,
tutor dev start
/api/v1/mfe_config
)4.1. Example:
<your_domain>/api/v1/mfe_config
4.2 Example:
<your_domain>/api/v1/mfe_config?mfe=mymfe
This API takes the information found in MFE_CONFIG and MFE_CONFIG_MYMFE key in site configuration ( admin > site configuration > your domain); and if you want to add information you just have to add something like this: