public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Feature #5370 User should be able to set the binary path for each database server
Date: Fri, 4 Jun 2021 12:29:11 +0100
Message-ID: <CA+OCxoxRfU67EMKSv1G=p_Mk9s2urDCGd2WYJKnYBPYcmSRzKw@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDfHkqiYtmmMOPExWYs-kJSEMBb98AHh1PLEAE8b3x7q7w@mail.gmail.com>
References: <CANxoLDfRrBE4e0sFL7nt7GxRy4ALyjCMSwv_YC1RPq33uWBzwA@mail.gmail.com>
<CAM9w-_nYDWxCVOB6-xLu-Yqw79juoAKv1Jv7UJcSujphPK2hkg@mail.gmail.com>
<CANxoLDfe8H-E9qmO1KYiRVodpBRWPcEmxKKKwPXQ4MDShE9XPQ@mail.gmail.com>
<CA+OCxozFY0Vs_KmhmCR0ranY=ti-Qc5YU1vM1nPsj0erQDnWvQ@mail.gmail.com>
<CANxoLDeeYbPuZc4DPdsCMEEiPdwopM+cq0VOwsrahz1wvjZQNg@mail.gmail.com>
<CA+OCxozkNdeH3RmMm-wS_YLNScRn7fUqZ0zEf+p9MVeWR_u+tQ@mail.gmail.com>
<CANxoLDf82r-kZfctPF5kG1dePwgCEveL=XGo7anmZi9Vvjdb=Q@mail.gmail.com>
<CA+OCxozV34Ec_+VoxNfUL0ROdsuzmyqvo_wfosBUSw6OTssUGg@mail.gmail.com>
<CANxoLDc37ss+TA4o_yBahGUO+WWm9U7ZEZ-js9Pyb0ZWcKXpLw@mail.gmail.com>
<CA+OCxoz-28ghNLD3wq6XpwnY+uLf4Ve0OsL2o46TrE-w5PEjZw@mail.gmail.com>
<CANxoLDdFaVbEd3uk_JmNp83Djdm=gEVtdtTTVqGv4KhtBYPYvQ@mail.gmail.com>
<CA+OCxow9=HE6t2aacn+aJDj6N6zZbjwTWpn1UPMCUqAD-0tyVw@mail.gmail.com>
<CANxoLDfHkqiYtmmMOPExWYs-kJSEMBb98AHh1PLEAE8b3x7q7w@mail.gmail.com>
On Fri, Jun 4, 2021 at 9:59 AM Akshay Joshi <[email protected]>
wrote:
> Hi Dave
>
> On Fri, Jun 4, 2021 at 1:51 PM Dave Page <[email protected]> wrote:
>
>> Hi Akshay,
>>
>> On Thu, Jun 3, 2021 at 3:07 PM Akshay Joshi <
>> [email protected]> 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 <[email protected]> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> On Fri, May 21, 2021 at 10:01 AM Akshay Joshi <
>>>> [email protected]> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, May 21, 2021 at 1:01 PM Dave Page <[email protected]> wrote:
>>>>>
>>>>>> Hi Akshay,
>>>>>>
>>>>>> On Fri, May 21, 2021 at 8:03 AM Akshay Joshi <
>>>>>> [email protected]> 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
Attachments:
[image/png] Binary_Path.png (323.2K, 3-Binary_Path.png)
download | view image
[image/png] Screenshot 2021-05-21 at 2.16.57 PM.png (296.9K, 4-Screenshot%202021-05-21%20at%202.16.57%20PM.png)
download | view image
[image/png] Screenshot 2021-05-21 at 2.23.50 PM.png (293.2K, 5-Screenshot%202021-05-21%20at%202.23.50%20PM.png)
download | view image
[image/png] Validate_2.png (56.8K, 6-Validate_2.png)
download | view image
view thread (14+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Feature #5370 User should be able to set the binary path for each database server
In-Reply-To: <CA+OCxoxRfU67EMKSv1G=p_Mk9s2urDCGd2WYJKnYBPYcmSRzKw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox