Re: Feature #5370 User should be able to set the binary path for each database server

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Feature #5370 User should be able to set the binary path for each database server
Date: 2021-06-04 08:21:32
Message-ID: CA+OCxow9=HE6t2aacn+aJDj6N6zZbjwTWpn1UPMCUqAD-0tyVw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Akshay,

On Thu, Jun 3, 2021 at 3:07 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Dave/Hackers
>
> I have almost completed the implementation of this feature as per the
> discussion. Facing one wired alertify issue whenever we open the preference
> dialog and scroll to the bottom of any page and then open another alertify
> dialog (About) scroll bars automatically scroll to top
> https://redmine.postgresql.org/issues/6506.
>
> After creating a new RM #6506, I did some R&D and figure out that if
> create Preferences dialog as a modal dialog issue is resolved. So I think
> there should not be any problem to make the Preferences dialog as a modal
> dialog?
>

I'd much prefer to avoid using modals (I'd also like to get rid of
Alertify, but that's another issue altogether). I'm not sure this minor bug
really warrants using a modal - it's a trivial inconvenience, that occurs
in quite unusual circumstances.

>
> Please refer to the image below:
> [image: Validate_2.png]
>
> Is the above look good to you?
>

Yes.

>
> Regarding migrate and extend the configuration "DEFAULT_BINARY_PATHS" I
> have the following questions:
>
> - If "DEFAULT_BINARY_PATHS" is defined then, where to show this in the
> grid? Can we show it to the latest database server version and make it
> default?
>
> Set it for the correct version, not the latest. It should be easy enough
to figure out.

>
> - If you agree to the first point then what if the user has already
> define another path for the latest database server version, should we
> overwrite it?
>
> No - only set the per-version setting if it's empty.

Thanks.

>
>
> On Fri, May 21, 2021 at 2:46 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi Akshay,
>>
>> On Fri, May 21, 2021 at 10:01 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Fri, May 21, 2021 at 1:01 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> On Fri, May 21, 2021 at 8:03 AM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Dave/Hackers
>>>>>
>>>>> As per your suggestion, I have created a new Backform control
>>>>> "BinaryPathsGridControl" and two new BackgridCell (BackgridRadioCell and
>>>>> BackgridSelectFileCell). Please refer to the screenshot below:
>>>>> [image: Binary_Path.png]
>>>>>
>>>>> Are the above changes look good to you? The radio button will only
>>>>> be enabled when there is a path. Added validate button which will validate
>>>>> the Utilities (pg_dump, pg_dumpall, ...)
>>>>>
>>>>
>>>> Nice! Just a couple of comments:
>>>>
>>>> - I assume the browse button is removed in server mode as discussed?
>>>> Maybe we should add a config.py option to allow that behaviour to be
>>>> overridden if the admin doesn't care about sandboxing?
>>>>
>>>
>>> Yes, will take care that in server mode browse button should not be
>>> visible. if the config option "ENABLE_FILE_BROWSING" is set to true then
>>> only the browse button will be enabled in server mode.
>>>
>>
>> I think the name needs to be a little more specific. How about
>> ENABLE_BINARY_PATH_BROWSING?
>>
>>
>>>
>>>>
>>>> - I think we need some hint text. How about something like:
>>>>
>>>> Enter the directory in which the psql, pg_dump, pg_dumpall, and
>>>> pg_restore utilities can be found for the corresponding database server
>>>> version. The default path will be used for server versions that do not have
>>>> a path specified.
>>>>
>>>
>>> I have added the hint in two ways, please refer to the screenshots
>>> below and let me know your prefered one.
>>> [image: Screenshot 2021-05-21 at 2.16.57 PM.png]. [image:
>>> Screenshot 2021-05-21 at 2.23.50 PM.png]
>>>
>>
>> I think having the text below is better. It's more consistent with other
>> controls.
>>
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>

--
Dave Page
Blog: https://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2021-06-04 08:35:11 Re: pgAdmin 4 commit: Hardcoded 'itsdangerous' version to <=1.1.0, as the l
Previous Message Akshay Joshi 2021-06-04 07:19:26 Re: [patch][pgAdmin] RM1591 [Web Based] Grant Wizard should also add under package node