Environment variables configurable thru yaml file #394
Conversation
Update from head
Make the RELATIONSHIP/ROUTES/APPLICATION and VARIABLES configurable thru the .magento.app.yaml file to allow users to specify an alternative variable name that is used to get the configuration value. An example is the use of the ECE tools on a native Platform.sh environment where the variables used are prefixed by PLATFORM instead of MAGENTO_CLOUD.
|
|
There was a problem hiding this comment.
@vkerkhoff thank you for your contribution!
I left 1 comment regarding location of those new varaiables.
|
Please also fix failed tests |
Added extra section in the .magento.env.yaml that allows storing the new variable options Created a reader/processor for getting the options from the system section
|
Moved the environment variables options to a new system => variables section in the .magento.env.yaml file. Let me know if this setup fits the design wanted so I can also add the tests and check why some tests fail. |
|
@vkerkhoff this looks good, thanks! It looks like |
|
@shiftedreality Thanks! I added the |
src/Config/SystemConfigInterface.php
Outdated
| /** | ||
| * Option for enabling merging given configuration with default configuration | ||
| */ | ||
| const OPTION_MERGE = '_merge'; |
There was a problem hiding this comment.
This seems not necessary in system config
There was a problem hiding this comment.
Please also update the dist/.magento.env.yaml file: https://github.com/magento/ece-tools/blob/develop/dist/.magento.env.yaml
Also for completeness, please also move MAGENTO_CLOUD_MODE and MAGENTO_CLOUD_ENVIRONMENT
Thanks for the contribution!!!
Fixed comments from code review (doc blocks and code styling) Added MAGENTO_CLOUD_MODE and MAGENTO_CLOUD_ENVIRONMENT variables to be configurable Updated Unit Tests to cover changes
|
@billygilbert I updated the code with your comments and added some documentation in the dist/.magento.env.yaml file. @shiftedreality I added/fixed the unit tests, the only message I don't see how to resolve is the code coverage, if this needs some changes please give some input on what to add. |
|
@vkerkhoff code coverage is increasing by unit tests. Looks like it's already green |
|
@shiftedreality Thanks, I see it indeed. Funny that if I run the test locally I get 94.69% code coverage. |
|
@vkerkhoff I observed the same and do not have a clear explanation of this. Looks like it somehow depends on OS(I use Mac). |
| # system: # | ||
| # variables: # | ||
| # ENV_ENVIRONMENT: "MAGENTO_CLOUD_ENVIRONMENT" # | ||
| ####################################################################################################################### |
|
@bbatsche Thanks, I updated the requested changes |
|
QA approved |
shiftedreality
added
Progress: accept
|
@vkerkhoff thank you for your contribution! |
Description
Make the RELATIONSHIP/ROUTES/APPLICATION and VARIABLES configurable thru the .magento.app.yaml file to allow users to specify an alternative variable name that is used to get the configuration value.
An example is the use of the ECE tools on a native Platform.sh environment where the variables used are prefixed by PLATFORM instead of MAGENTO_CLOUD.
Manual testing scenarios
Add for example the following settings in .magento.env.yaml
stage:
global:
ENV_RELATIONSHIPS: "PLATFORM_RELATIONSHIPS"
ENV_ROUTES: "PLATFORM_ROUTES"
ENV_APPLICATION: "PLATFORM_APPLICATION"
ENV_VARIABLES: "PLATFORM_VARIABLES"
No automated tests yet, if feature idea is approved I will add them
Contribution checklist