Hi Akshay, I have updated the existing implementation as per suggestions. 1. Show servers in server groups in the dropdown. 2. Current selected connection in the new connection dropdown is now highlighted as selected. 3. Notification to the user before the change connection action. 4. If we connect to the server through a new connection dialog, the tree will use the same connection and it will not create a new connection. (In earlier implementation it was asking for the password even we have connected from a new connection dialog.)
PFA patch Regards, Nikhil Mohite. On Thu, Oct 8, 2020 at 11:39 AM Akshay Joshi <akshay.jo...@enterprisedb.com> wrote: > Thanks, patch applied. > > On Wed, Oct 7, 2020 at 12:11 PM Nikhil Mohite < > nikhil.moh...@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 < >> akshay.jo...@enterprisedb.com> wrote: >> >>> Hi Nikhil >>> >>> Please verify and remove async = false wherever possible. >>> >>> On Tue, Oct 6, 2020 at 5:24 PM Dave Page <dave.p...@enterprisedb.com> >>> wrote: >>> >>>> >>>> >>>> On Tue, Oct 6, 2020 at 12:51 PM Murtuza Zabuawala < >>>> murtuza.zabuaw...@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.jo...@enterprisedb.com> wrote: >>>>> >>>>>> Thanks, patch applied. >>>>>> >>>>>> On Thu, Oct 1, 2020 at 10:42 AM Nikhil Mohite < >>>>>> nikhil.moh...@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.jo...@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.moh...@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.jo...@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.moh...@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.jo...@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.moh...@enterprisedb.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* >
RM_3794_review_comments.patch
Description: Binary data
