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

From: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(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-07 06:41:41
Message-ID: CAOBg0AMU_yTSrVU6Z+Q0Je4PuE3ncALdDA3Q=QUjQeXiWuHS6w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

Attachment Content-Type Size
RM_3794_V7.patch application/octet-stream 11.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Nikhil Mohite 2020-10-08 05:47:36 RM-4232- Change what is shown by default in tab titles: Implementation suggestion.
Previous Message Akshay Joshi 2020-10-07 03:11:59 pgAdmin 4 commit: Fixed API test cases failed for PG 9.5 due to change