Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kQP7O-0000mP-Rf for pgadmin-hackers@arkaria.postgresql.org; Thu, 08 Oct 2020 06:09:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kQP7N-0007gS-Fn for pgadmin-hackers@arkaria.postgresql.org; Thu, 08 Oct 2020 06:09:33 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kQP7N-0007gK-1J for pgadmin-hackers@lists.postgresql.org; Thu, 08 Oct 2020 06:09:33 +0000 Received: from mail-io1-xd43.google.com ([2607:f8b0:4864:20::d43]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1kQP7J-0001R3-U1 for pgadmin-hackers@postgresql.org; Thu, 08 Oct 2020 06:09:31 +0000 Received: by mail-io1-xd43.google.com with SMTP id d197so5083674iof.0 for ; Wed, 07 Oct 2020 23:09:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qHD/F+5gVlRAsc5fEEj2qXwr8VOOBQNOQAGB1DYL39s=; b=hP0y9al3OWFK34jAF4G1WicN0hXY3d87YyLK3rZndLUI05AN6UaSOWZ7qPjgswQ2e7 OQvM2nd541qwQ7Od00X40ViwXq8JBmYTwaYSmfqUrnTcVFV9hLxcrtJ68F0ik9qoVWmw VPQnQvJx6BUa8DdnqFt8kcclIz1W239o4aGlOr4DHvE1m3beSR8U/WprOTysVCchxpL0 SISrpqybaEQgXcBo+5t4ACuRTQBIi2J0uXvcMy9OC6tvJ3VxjmhCN4vD1S9kQSQH8DAH 7a+PNq+uir2FRJKCLw28RFA0dNViXH3jY329tfINS/JD8jWKrKfudvbHSTofvyHgUpGJ TaFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qHD/F+5gVlRAsc5fEEj2qXwr8VOOBQNOQAGB1DYL39s=; b=Xw1xU+zD7ZDzxFiKA4mspNpScUz+mVhqdie+q8YVizhZgBfhsgP5QbUyUEvxCvCIuI qFL8d7IyJI7DNeKYvGPJ2BVJ1G/Vf9nBGhQK31HueT/kRySnXry6xCkODlhbmaBjN2qs gO9VSvhcCL93njpKlOjeDoM6KLGyHNg5DAZ4joLpQyNCahzmBHqS9RPk4cIC3RYcsUJn WAlh62O+5ei/DQkpI6LdCxwkTqWk7pLjGk0Vv/jwBqOj1tjC5DD8M76usR5K+NPyBiRz BB560tJgkULw3380JuBzFaW/o751kP8ValNdJVO4Frl2AhicRr201b5YxFlAQ0g2UcN3 PLKw== X-Gm-Message-State: AOAM530ZJ73tyPG21DLjjTL+AE9jtzyLZdpvJX1f99nKJgPQIJD1dzuz tZ9Nm21HiRYPo+ULIt8x2MSuw9lW0ojNsO0cizlvCijoXn4hezyIUzTEyYU51XzKlVy3fv2zTL0 Ier8WATLm+qub/WKjwaCKypX5xi+Bx+HwIlDoKiShMYhoFF6x7rvoaQW54EMqS+B3hhs8FmRosV 7Nm3rOVs736vw5+vQsTT96HZ47ZmbKX9IqxWhAr/OTpvALnqCDuinloPPZDA== X-Google-Smtp-Source: ABdhPJwpTZiBnc4jvemIOsEQ448UT/0tO9FcNNwMkH5OUjCPHjqArh2y7jYRH7IZ+KXbMvqqiwGbAIbaBTyq4UP0IuA= X-Received: by 2002:a02:7350:: with SMTP id a16mr5893454jae.53.1602137368880; Wed, 07 Oct 2020 23:09:28 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Thu, 8 Oct 2020 11:39:17 +0530 Message-ID: Subject: Re: [pgAdmin][RM3794]:Allow User to Change Database Connection from an Open Query Tool Tab To: Nikhil Mohite Cc: pgadmin-hackers , Murtuza Zabuawala Content-Type: multipart/alternative; boundary="000000000000c1026d05b122ac2a" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000c1026d05b122ac2a Content-Type: text/plain; charset="UTF-8" Thanks, patch applied. On Wed, Oct 7, 2020 at 12:11 PM Nikhil Mohite < nikhil.mohite@enterprisedb.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 > wrote: > >> Hi Nikhil >> >> Please verify and remove async = false wherever possible. >> >> On Tue, Oct 6, 2020 at 5:24 PM Dave Page >> wrote: >> >>> >>> >>> On Tue, Oct 6, 2020 at 12:51 PM Murtuza Zabuawala < >>> murtuza.zabuawala@enterprisedb.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.joshi@enterprisedb.com> wrote: >>>> >>>>> Thanks, patch applied. >>>>> >>>>> On Thu, Oct 1, 2020 at 10:42 AM Nikhil Mohite < >>>>> nikhil.mohite@enterprisedb.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.joshi@enterprisedb.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.mohite@enterprisedb.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.joshi@enterprisedb.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.mohite@enterprisedb.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.joshi@enterprisedb.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.mohite@enterprisedb.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Team, >>>>>>>>>>>> >>>>>>>>>>>> Regarding RM-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* >>>>>>>>>>>> *Mob.No: +91-7798364578.* >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> *Thanks & Regards* >>>>>>>>>>> *Akshay Joshi* >>>>>>>>>>> *pgAdmin Hacker | Sr. Software Architect* >>>>>>>>>>> *EDB Postgres * >>>>>>>>>>> >>>>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> *Thanks & Regards* >>>>>>>>> *Akshay Joshi* >>>>>>>>> *pgAdmin Hacker | Sr. Software Architect* >>>>>>>>> *EDB Postgres * >>>>>>>>> >>>>>>>>> *Mobile: +91 976-788-8246* >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *Thanks & Regards* >>>>>>> *Akshay Joshi* >>>>>>> *pgAdmin Hacker | Sr. Software Architect* >>>>>>> *EDB Postgres * >>>>>>> >>>>>>> *Mobile: +91 976-788-8246* >>>>>>> >>>>>> >>>>> >>>>> -- >>>>> *Thanks & Regards* >>>>> *Akshay Joshi* >>>>> *pgAdmin Hacker | Sr. Software Architect* >>>>> *EDB Postgres * >>>>> >>>>> *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 * >> >> *Mobile: +91 976-788-8246* >> > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Sr. Software Architect* *EDB Postgres * *Mobile: +91 976-788-8246* --000000000000c1026d05b122ac2a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks, patch applied.

On Wed, Oct 7, 2020 at 12:11 PM Nikh= il Mohite <nikhil.mohi= te@enterprisedb.com> wrote:
Hi Akshay,

I che= cked the implementation and found 2 locations which I missed in the last pa= tch to remove async: False.
I have removed all occurrences of asy= nc: False now also=C2=A0added missing loader in required places.
=
PFA updated the patch for the same.

Regards,
Nikhil Mohite.

On Tue, Oct 6, 2020 at 6:19 PM Aksh= ay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Nikhil=C2=A0

=
Please verify and remove async =3D false wherever possible.

On Tue, Oct 6, 2020 at 5:24 PM Dave Page <dave.page@enterprisedb.com> wrote= :


On Tue, Oct 6, 2020 at 12:51 PM Murtuza Zabuawala &= lt;= murtuza.zabuawala@enterprisedb.com> wrote:
Hi Akshay,

We have used aysnc=3DFals= e in most ajax calls with this feature, It is causing=C2=A0UI hang in case = of slow server response.
You can try adding a time.sleep() call at the python s= ide response and check the UI hang, I think we should avoid sync calls as m= uch as possible.

I consider a s= ync ajax call to be a bug.
=C2=A0


--
Regards,
Murtuza Zabuawala
<= /div>
EDB
POWER TO POSTGRES=

<= /div>
O= n Thu, Oct 1, 2020 at 1:31 PM Akshay Joshi <akshay.joshi@enterprisedb.com>= ; wrote:
Thanks, patch applied.

On Thu, Oct 1, 2020 at 10:42 AM Nikhil = Mohite <nikhil.mohite@enterprisedb.com> wrote:
Hi Akshay,

<= /div>
I have=C2=A0resolved the sonarQube issues, PFA updated patch for = the same.


Regards,
Nikhil= Mohite.


On Tue, Sep 29, 2020 at 11:31 AM Akshay Joshi= <aks= hay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Nikhil

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 &l= t;nikhi= l.mohite@enterprisedb.com> wrote:
Hi Akshay,

I have resolved code conflict issues and sonarqube issues.
PFA u= pdated patch.

Regards,
Nikhil Mohite.

On Mon, Sep 28, 2020 at 5:58 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Nikhil

The patch is not applying,= rebase, and send it again. Please check your code should not create any ne= w SonarQube issues.

On Mon, Sep 28, 2020 at 11:20 AM Nikhil Mohite <= ;nikhil= .mohite@enterprisedb.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 Josh= i <ak= shay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Nikhil

Following are the initial review comments:
  • Open View/Edit da= ta on any table and click on the same database connection and then click on= the Execute button. Got "get_primary_keys() takes 1 positional argume= nt 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 ba= ckend, no action required in this case.
  • The role option is missing = from the "connect to server" dialog.
  • The Password field s= hould not be there on the "connect to server" dialog. Sometimes w= e saved the password so asking a password every time is not correct. Check = the pgAdmin 3 behavior.
Code review still remains.

On T= hu, Sep 17, 2020 at 4:15 PM Nikhil Mohite <nikhil.mohite@enterprisedb.com&g= t; wrote:
Hi Team,

Regarding RM-3794 allow the=C2=A0u= ser to change the database connection from an open query tool:
I = have implemented the feature and also added documentation for it.

PFA patch.

--
<= div dir=3D"ltr">Thanks & Regards,=
Nikhil Mohite<= /b>
Software Engineer.
Mob.No: +91-7798364578.=


--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Dave Page
VP & Chief Architect, Database Infr= astructure
EDB: http://www.enterprisedb.com

Blog: http://pgsnake.blogspot.com
Twitter: @= pgsnake


--
Thank= s & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Ar= chitect
EDB Postgres=
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmi= n Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246
=
--000000000000c1026d05b122ac2a--