-
Notifications
You must be signed in to change notification settings - Fork 7
Enforce the load order of components defined in env.local #304
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
Changes from all commits
096509a
25daaea
a91b7dc
d91dba0
d4e7ea4
fb92700
8d80642
8272c93
11d0279
81a2e41
d580911
94ffced
343e26d
21825f9
d747892
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| name: Unit tests | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| jobs: | ||
| test: | ||
| if: github.event.pull_request.draft == false | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Set up python for testing | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: "3.11" | ||
| cache: 'pip' | ||
| - name: Install python test dependencies | ||
| run: | | ||
| pip install -r ./tests/requirements.txt | ||
| - name: Test with pytest | ||
| run: | | ||
| pytest ./tests/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
|
|
||
| # Changes | ||
|
|
||
| [//]: # (NOTES:) | ||
|
|
@@ -16,6 +17,94 @@ | |
|
|
||
| [//]: # (list changes here, using '-' for each new entry, remove this when items are added) | ||
|
|
||
| [1.25.4](https://github.com/bird-house/birdhouse-deploy/tree/1.25.4) (2023-04-12) | ||
| ------------------------------------------------------------------------------------------------------------------ | ||
|
|
||
| ## Fixes | ||
| - Enforce the load order of components defined in env.local | ||
|
|
||
| Extra components defined in the `EXTRA_CONF_DIRS` variables were being loaded before the dependant components | ||
| defined in the `COMPONENT_DEPENDENCIES` variables in each default.env file. This meant that if an extra component | ||
| was meant to override some setting defined in a dependant component, the setting would not be overridden by the | ||
| extra component. | ||
|
|
||
| This change enforces the following load order rules: | ||
|
|
||
| - components defined in `DEFAULT_CONF_DIRS` are loaded before those in `EXTRA_CONF_DIRS` | ||
| - components are loaded in the order they appear in either `DEFAULT_CONF_DIRS` or `EXTRA_CONF_DIRS` | ||
| - components that appear in `COMPONENT_DEPENDENCIES` variable are immediately loaded unless they have already been | ||
| loaded | ||
|
|
||
| For example, with the following files in place: | ||
|
|
||
| ```shell | ||
| # env.local | ||
| DEFAULT_CONF_DIRS=" | ||
| ./config/twitcher | ||
| ./config/project-api | ||
| ./config/magpie | ||
| " | ||
| EXTRA_CONF_DIRS=" | ||
| ./optional-components/generic_bird | ||
| ./components/cowbird | ||
| " | ||
|
|
||
| # config/twitcher/default.env | ||
| COMPONENT_DEPENDENCIES=" | ||
| ./config/magpie | ||
| " | ||
| # optional-components/generic_bird/default.env | ||
| COMPONENT_DEPENDENCIES=" | ||
| ./config/wps_outputs-volume | ||
| " | ||
| ``` | ||
|
|
||
| the load order is: | ||
|
|
||
| - ./config/magpie (loaded as a dependency of twitcher, not loaded a second time after project-api) | ||
| - ./config/twitcher | ||
| - ./config/project-api | ||
| - ./config/wps_outputs-volume (loaded as a dependency of generic_bird) | ||
| - ./optional-components/generic_bird | ||
| - ./components/cowbird | ||
|
|
||
| This load order also applies to the order that docker-compose-extra.yml files are specified. If a component also | ||
| includes an override file for another component (eg: ./config/finch/config/proxy/docker-compose-extra.yml overrides | ||
| ./config/proxy/docker-compose-extra.yml), the following additional load order rules apply: | ||
|
|
||
| - if the component that is being overridden has already been loaded, the override file is loaded immediately | ||
| - otherwise, the override files will be loaded immediately after the component that is being overridden has been loaded | ||
|
|
||
| For example, with the following files in place: | ||
|
|
||
| ```shell | ||
| # env.local | ||
| DEFAULT_CONF_DIRS=" | ||
| ./config/finch | ||
| ./config/proxy | ||
| " | ||
| ``` | ||
| ```yaml | ||
| # config/proxy/docker-compose-extra.yml | ||
| ... | ||
| # config/finch/docker-compose-extra.yml | ||
| ... | ||
| # config/finch/config/proxy/docker-compose-extra.yml | ||
| ... | ||
| ``` | ||
|
|
||
| the docker compose files will be loaded in the following order: | ||
|
|
||
| - config/finch/docker-compose-extra.yml | ||
| - config/proxy/docker-compose-extra.yml | ||
| - config/finch/config/proxy/docker-compose-extra.yml | ||
|
Comment on lines
+98
to
+100
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to contradict what the test evaluates, or there is something I missed?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finch doesn't have proxy as an explicit dependency (it is not included in the COMPONENT_DEPENDENCIES variable defined in config/finch/default.env) so proxy is not loaded before finch. Because of this, the order that the components appear in the DEFAULT_CONF_DIRS list is respected (finch is before proxy). However, the file that overrides proxy settings (config/finch/config/proxy/docker-compose-extra.yml) can't be loaded until after both finch and proxy are loaded, since it could potentially override settings in either one, so it is loaded as soon as both of finch an proxy are loaded. From the CHANGES.md file, here is the relevant explanation:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So in the tests, |
||
|
|
||
| - Add tests to ensure override capabilities are preserved which allows all default | ||
| behaviors of the platform can be customized. | ||
|
|
||
| See [birdhouse/README.rst](birdhouse/README.rst) for instruction to run the | ||
| tests. | ||
|
|
||
| [1.25.3](https://github.com/bird-house/birdhouse-deploy/tree/1.25.3) (2023-04-12) | ||
| ------------------------------------------------------------------------------------------------------------------ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 1.25.3 2023-04-12T21:49:10Z | ||
| 1.25.4 2023-04-12T22:02:31Z |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
|
|
||
| # add any component that this component requires to run | ||
| COMPONENT_DEPENDENCIES=" | ||
| $COMPONENT_DEPENDENCIES | ||
| ./config/geoserver | ||
| " |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| # add any component that this component requires to run | ||
| COMPONENT_DEPENDENCIES=" | ||
| $COMPONENT_DEPENDENCIES | ||
| ./config/magpie | ||
| ./config/thredds | ||
| " |
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.
Very nice delayed load ! Components do not have to know the load ordering amount themselves.