Skip to content
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

fix overriding configuration #144

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rommelfreddy
Copy link
Contributor

@rommelfreddy rommelfreddy commented Jul 16, 2024

This fixes an issue that if you save the configuration within the administration, the sentry error reporting got disabled, because of the setting sentry/environment/enabled

this PR will add another setting with which it is possible to enable to override.
Only if the override is enabled, it is possible to disable the Sentry error tracking.

Also reworked the collectModuleConfig within the config-helper to make sure that values, which are null or empty, do not override the deployment configuration.

I added canRestore to the system.xml to make sure that no empty values got stored into the database.

I removed the TableNotFoundException-Handling, because i do not see a case when this could happen.

@indykoning
Copy link
Member

Awesome i can't wait to get this merged!

The TableNotFoundException had been introduced in #125
I could personally reproduce this in the few cases of setting up the project with sentry already installed, or in deployment pipelines where the code would be built but no database connection could be available.

Could you try the steps in that PR and see if it is no longer an issue with your current code? 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants