Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab
Date: 2020-10-08 06:09:17
Message-ID: CANxoLDd1M0kY4jM5Me=cUpejFt5cAQV9Uz8Sv6i7C3LgYJOLJg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, patch applied.

On Wed, Oct 7, 2020 at 12:11 PM Nikhil Mohite <
nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:

> Hi Akshay,
>
> I checked the implementation and found 2 locations which I missed in the
> last patch to remove async: False.
> I have removed all occurrences of async: False now also added missing
> loader in required places.
>
> PFA updated the patch for the same.
>
> Regards,
> Nikhil Mohite.
>
> On Tue, Oct 6, 2020 at 6:19 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
> wrote:
>
>> Hi Nikhil
>>
>> Please verify and remove async = false wherever possible.
>>
>> On Tue, Oct 6, 2020 at 5:24 PM Dave Page <dave(dot)page(at)enterprisedb(dot)com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Oct 6, 2020 at 12:51 PM Murtuza Zabuawala <
>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>> We have used aysnc=False in most ajax calls with this feature, It is
>>>> causing UI hang in case of slow server response.
>>>> You can try adding a time.sleep() call at the python side response and
>>>> check the UI hang, I think we should avoid sync calls as much as possible.
>>>>
>>>
>>> I consider a sync ajax call to be a bug.
>>>
>>>
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> *EDB*
>>>> *POWER TO POSTGRES*
>>>> https://www.edbpostgres.com
>>>>
>>>>
>>>> On Thu, Oct 1, 2020 at 1:31 PM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Thanks, patch applied.
>>>>>
>>>>> On Thu, Oct 1, 2020 at 10:42 AM Nikhil Mohite <
>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Akshay,
>>>>>>
>>>>>> I have resolved the sonarQube issues, PFA updated patch for the same.
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Nikhil Mohite.
>>>>>>
>>>>>>
>>>>>> On Tue, Sep 29, 2020 at 11:31 AM Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Nikhil
>>>>>>>
>>>>>>> Your patch introduces 1 new Bug and 13 new code smells, please fix
>>>>>>> those and resend the patch.
>>>>>>>
>>>>>>> On Mon, Sep 28, 2020 at 7:31 PM Nikhil Mohite <
>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Akshay,
>>>>>>>>
>>>>>>>> I have resolved code conflict issues and sonarqube issues.
>>>>>>>> PFA updated patch.
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Nikhil Mohite.
>>>>>>>>
>>>>>>>> On Mon, Sep 28, 2020 at 5:58 PM Akshay Joshi <
>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Nikhil
>>>>>>>>>
>>>>>>>>> The patch is not applying, rebase, and send it again. Please check
>>>>>>>>> your code should not create any new SonarQube issues.
>>>>>>>>>
>>>>>>>>> On Mon, Sep 28, 2020 at 11:20 AM Nikhil Mohite <
>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Akshay,
>>>>>>>>>>
>>>>>>>>>> I have resolved all the review comments and also updated the test
>>>>>>>>>> cases as per the new implementation.
>>>>>>>>>>
>>>>>>>>>> PFA updated patch.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Mon, Sep 21, 2020 at 5:24 PM Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Nikhil
>>>>>>>>>>>
>>>>>>>>>>> Following are the initial review comments:
>>>>>>>>>>>
>>>>>>>>>>> - Open View/Edit data on any table and click on the same
>>>>>>>>>>> database connection and then click on the Execute button. Got
>>>>>>>>>>> "get_primary_keys() takes 1 positional argument but 2 were given" error.
>>>>>>>>>>> - In my opinion, we should hide the option to change the
>>>>>>>>>>> database connection for View/Edit Data.
>>>>>>>>>>> - If the user clicks on the same database connection
>>>>>>>>>>> multiple times then no need to change the backend connection and
>>>>>>>>>>> transaction id. Add validation at the backend, no action required in this
>>>>>>>>>>> case.
>>>>>>>>>>> - The role option is missing from the "connect to server"
>>>>>>>>>>> dialog.
>>>>>>>>>>> - The Password field should not be there on the "connect to
>>>>>>>>>>> server" dialog. Sometimes we saved the password so asking a password every
>>>>>>>>>>> time is not correct. Check the pgAdmin 3 behavior.
>>>>>>>>>>>
>>>>>>>>>>> Code review still remains.
>>>>>>>>>>>
>>>>>>>>>>> On Thu, Sep 17, 2020 at 4:15 PM Nikhil Mohite <
>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Team,
>>>>>>>>>>>>
>>>>>>>>>>>> Regarding RM-3794 <https://redmine.postgresql.org/issues/3794>
>>>>>>>>>>>> allow the user to change the database connection from an open query tool:
>>>>>>>>>>>> I have implemented the feature and also added documentation for
>>>>>>>>>>>> it.
>>>>>>>>>>>>
>>>>>>>>>>>> PFA patch.
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> *Thanks & Regards,*
>>>>>>>>>>>> *Nikhil Mohite*
>>>>>>>>>>>> *Software Engineer.*
>>>>>>>>>>>> *EDB Postgres* <https://www.enterprisedb.com/>
>>>>>>>>>>>> *Mob.No: +91-7798364578.*
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Thanks & Regards*
>>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>>>
>>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Thanks & Regards*
>>>>>>>>> *Akshay Joshi*
>>>>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>
>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Thanks & Regards*
>>>>>>> *Akshay Joshi*
>>>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>
>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>> *pgAdmin Hacker | Sr. Software Architect*
>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>
>>> --
>>> Dave Page
>>> VP & Chief Architect, Database Infrastructure
>>> EDB: http://www.enterprisedb.com
>>>
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Sr. Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Sr. 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 Aditya Toshniwal 2020-10-08 08:01:00 [pgAdmin][patch] Packages security fixes
Previous Message Akshay Joshi 2020-10-08 06:08:47 pgAdmin 4 commit: Fixed the following issues for RM 3794: