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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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:58:54
Message-ID: CANxoLDfHkqiYtmmMOPExWYs-kJSEMBb98AHh1PLEAE8b3x7q7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave

On Fri, Jun 4, 2021 at 1:51 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

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

This is very annoying for Binary Path when the user selects the binary
path and tries to validate or try to select 'Set as default', it scrolls to
the top. We need to fix it by making Preferences dialog as a modal dialog.

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

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2021-06-04 11:29:11 Re: Feature #5370 User should be able to set the binary path for each database server
Previous Message Dave Page 2021-06-04 08:35:11 Re: pgAdmin 4 commit: Hardcoded 'itsdangerous' version to <=1.1.0, as the l