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

Implement Strict Config Parsing #807

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AndrewJackson2020
Copy link
Contributor

Currently when pgcat passes toml config files upon encountering any misspelled or unknown fields they are simply ignored. This is not ideal as a user could simply misspell a field, pgcat seemingly runs normally, and it could take a bit of debugging unknown behavior before the user realized they misspelled a field. Contrast that scenario with the behavior of this PR where a misspelled field will not allow pgbouncer to run until the config is fixed, much more explicit and causes problems to be caught earlier.

Some potential issues:

  • This is a breaking change, there could be pgcat instances in the wild with misspelled or erroneous config fields that run fine with older versions but would not work with this patch until said fields are fixed/removed
  • reload functionality will have to handle errors differently from startup functionality. Ideally on reload a log line is generated and if being run from the admin console a notify message is sent to the user but no change is made to the running process.

@AndrewJackson2020 AndrewJackson2020 changed the title Implemented strict config Implement Strict Config Parsing Sep 8, 2024
@AndrewJackson2020
Copy link
Contributor Author

Tested the reload functionality and can confirm that the server does not crash on reload of an invalid config. Had to modify the admin console reload command to warn the user that the reload command failed instead of returning an error but it seems to work.

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.

1 participant