| From: | Andrew Jackson <andrewjackson947(at)gmail(dot)com> |
|---|---|
| To: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Roman Khapov <rkhapov(at)yandex-team(dot)ru>, niushiji(at)gmail(dot)com, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Subject: | Re: Add ldapservice connection parameter |
| Date: | 2026-04-07 16:20:20 |
| Message-ID: | CAKK5BkEsKEnPifH10hXsXt7Z7qN2PPy6i64TH0A9yRr=mT=TtA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
> Oh, I didn't look at that. If ldapServiceLookup() already appends a
> meaningful error message, there is no need to add another one.
Updated patch to only append an error message when rc from
ldapServiceLookup is 2. This is the only non zero return of
ldapServiceLookup that does not append an error message because it is
meant to signal that the lookup was not successful but we can continue
parsing the service file. In this case I think it makes sense to raise
an error and append an error message. Also increased the specificity
of the test assertions.
> That change does not really belong to the patch, but is alright by me.
I added this because now that there is a connection parameter that
begins with ldap it could cause a confusing error message if the user
tries to use ldapserviceurl in a service file and are then given an
error message that roughly says 'invalid LDAP URL ".*" scheme must be
ldap://'.
Thanks,
Andrew Jackson
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Add-ldapservice-connection-parameter.patch | text/x-patch | 20.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nazir Bilal Yavuz | 2026-04-07 16:27:31 | Re: Upload only the failed tests logs to the Postgres CI (Cirrus CI) |
| Previous Message | Nazir Bilal Yavuz | 2026-04-07 16:18:49 | Upload only the failed tests logs to the Postgres CI (Cirrus CI) |