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

On Fri, Jun 4, 2021 at 9:59 AM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

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

OK.

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

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

EDB: https://www.enterprisedb.com