opengisch / solocator

GNU General Public License v3.0
0 stars 1 forks source link

Improvement for PostgreSQL connection options #31

Closed schmandr closed 5 years ago

schmandr commented 5 years ago

I actually think things mentioned in #30 could be simplified if we solved it like in the "Create a new PostGIS connection" dialog in QGIS: postgis_connection It contains input fields for Service, Host, Port, and the Database. So instead of forcing the user to choose between connecting using a service name or using host/port/database, we could leave it up to the user which fields to provide.

Just providing the service name and leaving the other fields emtpy (this must be the default) is perfectly fine. Then, an advanced user could add any of Host, Port, or Database which would override the values stored in the PostgreSQL service file (which is the QGIS default behaviour). Or he could leave the Service name empty and just specify Host, Port, and Database for defining a completely independent DB connection.

(Opposed to the standard QGIS dialog, for SoLocator we would omit the "Name" and "SSL mode" fields, and the "Port" field should be empty by default.)

What do you think of it?

3nids commented 5 years ago

I think the plugin benefits from a simple default configuration which is possible because end users work in a predefined environment.

At the origin there was only the auth config. PG service were added to give more freedom. Before making the configuration too complete, it would be good to define use cases to identify the different possible connection scenarios.

andreasneumann commented 5 years ago

Use case:

3nids commented 5 years ago

therefore, a hidden setting would be better appropriate no? we have this already, see https://github.com/opengisch/solocator/blob/master/README.md

andreasneumann commented 5 years ago

I guess a hidden setting for our SoGIS admin users would be acceptable (for testing and integration environment), however, changing the servicename is not enough.

We need to be able to have combinations of servicename (e.g. pub|edit|test) and hostnames (e.g. geodb|geodb-t|geodb-i).

All of the above mentioned hosts have the pub|edit|test dbs. Our service configuration points to geodb.

@schmandr told me that it is possible to have a service name in the psql|QGIS connection string and even if the service configuration (pg_service.conf) already has a host in the definition, the one provided in addition as host in the connection string has precedence. So a connection string could look like: "service=pub host=geodb ...."

So I guess we would need an additional hidden "pg_host" setting, where the default is set to geodb.

This way, a SoGIS test user can use the servicename "pub" and then overwrite the hostname to geodb-t in the hidden setting for testing a different host (e.g. test host).

@schmandr can you please comment and confirm that what I have written is correct?

3nids commented 5 years ago

We need to be able to have combinations of servicename (e.g. pub|edit|test) and hostnames (e.g. geodb|geodb-t|geodb-i)

How? A service usually contains the host, no? Or by servicename, you meant DB name?

So I guess we would need an additional hidden "pg_host" setting, where the default is set to geodb.

At the moment, we have

The credentials can be given via configurable authcfg or through the service.

So, questions are: 1) What should be configurable? 2) Where? a) hidden setting b) advanced config in the options c) layer loading options (CTRL+click)

It is there already.

andreasneumann commented 5 years ago

How? A service usually contains the host, no? Or by servicename, you meant DB name?

A service configuration can contain any part or combination of a connection string, incl. authentication information.

If a servicename is provided and in addition, a definition of a connection string, that latter one can overwrite the definition in the service definition.

I did not know about this either before I started in Solothurn, but it is really totally flexible, but also makes it a bit difficult UI wise.

I'll talk again to @schmandr - but I think part of the configuration that only concerns admin or test users can go into the hidden settings.

3nids commented 5 years ago

should be fixed in d1db29b463916a2208f25ae4d9a5bf8483cacb6b and mainly 1145e0b7c79d80d053f41da0bd33201d9fb65133