-
Notifications
You must be signed in to change notification settings - Fork 47
feat(update): Introduce user-specific config files to prevent merge conflicts #34
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(update): Introduce user-specific config files to prevent merge conflicts #34
Conversation
…onflicts - Added and as sample configuration files. - Removed and from the repository. - Updated to exclude user-specific config files. - Modified to allow missing config files and fall back to sample files if necessary. - Updated README with instructions on copying sample config files. Signed-off-by: Fortune-Ndlovu <[email protected]>
Signed-off-by: Fortune-Ndlovu <[email protected]>
/cc @rm3l @kadel @benwilcock |
As I was concerned that this would not work OOTB without copying the app-config.yaml.sample and dynamic-plugins.yaml.sample files, I was thinking of another approach that could probably help here. Since we control the entrypoint scripts used by both the install-dynamic-plugins and rhdh containers, and that we are mounting the whole I mean, with a new
We could do the following:
if [ -f "configs/dynamic-plugins/dynamic-plugins.override.yaml" ]; then
echo "Using dynamic-plugins.override.yaml file"
ln -sf /opt/app-root/src/configs/dynamic-plugins/dynamic-plugins.override.yaml /opt/app-root/src/dynamic-plugins.yaml
else
ln -sf /opt/app-root/src/configs/dynamic-plugins/dynamic-plugins.yaml /opt/app-root/src/dynamic-plugins.yaml
fi
USER_APP_CONFIG="configs/app-config/app-config.local.yaml"
extraCliArgs=""
if [ -f "$USER_APP_CONFIG" ]; then
extraCliArgs="--config $USER_APP_CONFIG"
fi
node packages/backend --no-node-snapshot \
--config app-config.yaml \
--config app-config.example.yaml \
--config app-config.example.production.yaml \
--config $DYNAMIC_PLUGINS_CONFIG \
--config configs/app-config/app-config.yaml $extraCliArgs This way, users would just need to create their Just like when working with a local Backstage, this would allow users to append their Thoughts? @kadel @benwilcock @Fortune-Ndlovu |
…red. Signed-off-by: Fortune-Ndlovu <[email protected]>
…ctured configs/ layout Signed-off-by: Fortune-Ndlovu <[email protected]>
Thanks, Armel for the great suggestion, I’ve now implemented this approach! Refactored
Updated
Removed the need to manually copy Updated |
…ailure Signed-off-by: Fortune-Ndlovu <[email protected]>
CI unhappy
The HTTP 200 check step timed out after 30 retries meaning: |
…he image. AND made the default app-onfig file from this repo to be last Signed-off-by: my-name <my-email>
…milar to the rhdh repo. Also Ignore dynamic-plugins.override.yaml and extra-files/* BUT Keep legacy files (e.g., dynamic-plugins.yaml, github-app-credentials.example.yaml) ignored for backward compatibility. Signed-off-by: my-name <my-email>
…mic-plugins.override.yaml file instead. So dynamic-plugins.local.yaml is not needed. Signed-off-by: my-name <my-email>
…l' but also copy it to 'configs/github-app-credentials.example.yaml' for backward compatibility and made sure that 'configs/github-app-credentials.example.yaml' is git-ignored. MOREOVER Keep 'configs/dynamic-plugins/dynamic-plugins.yaml' but also copy it to 'configs/dynamic-plugins.yaml', for backward compatibility and made sure that configs/dynamic-plugins.yaml is git-ignored. MOREOVER keep 'configs/app-config/app-config.yaml' and copy it to configs/app-config.local.yaml, for backward compatibility? Just make sure that configs/app-config.local.yaml is git-ignored. Signed-off-by: my-name <my-email>
…sting users nor create any Git conflicts. Signed-off-by: my-name <my-email>
… break existing users nor create any Git conflicts. Signed-off-by: my-name <my-email>
/cc @rm3l @kadel @benwilcock for review please! |
…c-plugins.yaml can be loaded if present as well. Signed-off-by: my-name <my-email>
…expand the string into proper CLI flags instead of passing it as one long argument. Signed-off-by: my-name <my-email>
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.
missing example files
@kadel, do we need example files? I feel like adding additional files may add additional complexity for new users |
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.
do we need example files? I feel like adding additional files may add additional complexity for new users
I agree it could be helpful to have a few example files.
So I'd suggest adding a configs/app-config/app-config.local.example.yaml
with a GH integration referencing the configs/extra-files/github-app-credentials.example.yaml
file, along with a configs/dynamic-plugins/dynamic-plugins.override.example.yaml
file.
I'm convinced about the opposite. Almost every user will need We are already using the pattern of the example, configs that user's copies with |
…e the copy if they wish. Signed-off-by: my-name <my-email>
2376639
to
3576b66
Compare
Signed-off-by: my-name <my-email>
…gins Signed-off-by: my-name <my-email>
…entials Signed-off-by: my-name <my-email>
…amic-plugins.override.yaml Signed-off-by: my-name <my-email>
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.
LGTM with the review comments addressed. I've also checked that the approach here remains backward-compatible.
/lgtm
…onfiguration This is an addendum to redhat-developer#34
Description
This PR introduces a new convention for user-specific config overrides using a structured directory layout inside the existing
configs/
folder. These override files are.gitignore
'd and will automatically be loaded by the system at runtime.New directory structure
Entrypoint updates
fixes.sh
(install-dynamic-plugins container):dynamic-plugins.override.yaml
as the active config, falling back to the default otherwise.wait-for-plugins.sh
(rhdh container):app-config.local.yaml
and appends it to the list of configs passed to thenode packages/backend
startup command.Git ignores
Updated
.gitignore
to:*.local.yaml
and override filesextra-files/
, except thegithub-app-credentials.example.yaml
Result
Users can now:
*.local.yaml
and override files that are auto-loaded at runtime.Which issue(s) does this PR fix or relate to
PR acceptance criteria
How to test changes / Special notes to the reviewer