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

Connect using Connection Service File #87

Merged

Conversation

feikesteenbergen
Copy link

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.

Feike Steenbergen and others added 2 commits June 22, 2017 13:39
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=<name> format we delegate all the
resolving of service file
locations to libpq.
@alexeyklyukin
Copy link
Contributor

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).

Avoid using the same value under different names.
Avoid unnecessary abbreviations
@feikesteenbergen
Copy link
Author

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

feikesteenbergen@bed388c

$ pg_view --help
[...]
  -s SERVICE, --service=SERVICE
                        name of the service to monitor

@alexeyklyukin
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

👍

1 similar comment
@a1exsh
Copy link
Member

a1exsh commented Jun 30, 2017

👍

@alexeyklyukin alexeyklyukin merged commit 17dd050 into zalando:master Jun 30, 2017
@alexeyklyukin
Copy link
Contributor

Thanks @feikesteenbergen

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