zalando / pg_view

Get a detailed, real-time view of your PostgreSQL database and system metrics
https://pypi.python.org/pypi/pg-view
Other
494 stars 49 forks source link

Connect using Connection Service File #87

Closed feikesteenbergen closed 7 years ago

feikesteenbergen commented 7 years ago

For those who already have their Connection Service Files setup, it is useful not to have to specify the configuration file. By connection using the service= format we delegate all the resolving of service file locations to libpq.

Requires issue #85 to be fixed.

alexeyklyukin commented 7 years ago

It breaks the functionality of picking up a specific instance among the auto-detected one, i.e. perhaps you have 3 clusters A,B and C and when you run pg_view it shows all of them, but you really need to monitor only A. Right now, you can say -i A, and pg_view shows only cluster 'A'.

With the proposed changes it will complain if there is no record for A in pg_service.conf, because it would always and unconditionally interpret it as a service name, unless there is a configuration file.

I'd propose adding some other parameter to point specifically to a service record.

Also, we probably want to give priority to the case where the option.host is set, as it is more specific than the name of the service (line 210 should be swapped with line 207).

feikesteenbergen commented 7 years ago

What about introducing another flag to specify the service? The gist would be something like this:

https://github.com/feikesteenbergen/pg_view/commit/bed388c4f6edc81eeb4190a95151ef22b55e624c

$ pg_view --help
[...]
  -s SERVICE, --service=SERVICE
                        name of the service to monitor
alexeyklyukin commented 7 years ago

I've used exactly -s in the PR I've created, but it acts as a modifier to -i, enabling pg_view to consider looking at the services file (so you have to specify both) and using -i to specify the individual service. Perhaps that looks like too much to type, but I thought that giving both -i and -s the role to chose instances would be confusing.

feikesteenbergen commented 7 years ago

I pulled in those changes into this PR. I think the way it currently is is fine - it doesn't interfere with current usage of pg_view, but it does allow me (and others in my organization) to not have to specify the service file all the time.

alexeyklyukin commented 7 years ago

👍

a1exsh commented 7 years ago

:+1:

alexeyklyukin commented 7 years ago

Thanks @feikesteenbergen