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

Allow configuration through config file and from args #153

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

Conversation

SoulKyu
Copy link

@SoulKyu SoulKyu commented Jun 4, 2024

Hello,

Ive implemented a little piece of code that allow to use a configFile for configuration.

Actually you can still provide configuration from args and now from configFile also.

It doesn't do anything if you didn't provide the --config.file so its safe for existing deployment.

A configuration can look like :

{
  "pgBouncer.connectionString": "postgres://username:password@hostname:port/dbname?sslmode=disable",
  "web.telemetry-path": "/metrics",
  "pgBouncer.pid-file": "/path/to/pgbouncer.pid"
}

Its a small modification, actually i think another config / args implementation like viper / kong should be used instead of kingpin which is not realy maintained anymore.
(in my point of view, using a lib for a little project like this is overkill and maybe a simple implementation could be better).

For security consideration, i think my PR would be very benefit as now credentials would be in a config-file instead of args.

Hope you will agree to merge :)

Thanks you

@stanhu
Copy link
Contributor

stanhu commented Jul 26, 2024

Thanks! Could you resolve the conflicts and add some documentation to README.md?

Comment on lines +37 to +39
ConnectionString string `json:"pgBouncer.connectionString"`
MetricsPath string `json:"web.telemetry-path"`
PidFilePath string `json:"pgBouncer.pid-file"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically use yaml config files in Prometheus. This is more flexible as it allows for things like comments.

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.

3 participants