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 1lgNM5-0007eS-HD for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 May 2021 08:03:01 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lgNM3-00045K-7U for pgadmin-hackers@arkaria.postgresql.org; Tue, 11 May 2021 08:02:59 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lgNM2-00045A-Vm for pgadmin-hackers@lists.postgresql.org; Tue, 11 May 2021 08:02:59 +0000 Received: from mail-il1-x133.google.com ([2607:f8b0:4864:20::133]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1lgNM0-0002cb-0i for pgadmin-hackers@postgresql.org; Tue, 11 May 2021 08:02:58 +0000 Received: by mail-il1-x133.google.com with SMTP id z1so8551246ils.0 for ; Tue, 11 May 2021 01:02:55 -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=60wPXQppWk8s475AbmBzw6IxHES+gXNHfdpR+hDaVkc=; b=WiQ2hTYmFAnoml5/uUpuAXASOYTe2kzlKeJviiOFnlRW4U1tiBoDMMIPviSYAYvQlZ 6Qj9UxpmLDxUTZ+EyG4HBs5KFRke4Lf3CtoOXcklkLM2s869v6vwmw7NkCLljEHMKY8K QxazQqZ2KwjW5CznJBPkt+vNO24jqlWAB893hP/XbEnlsocDE2o332Qbxy9fiAlO412V u/WcpXVwUd5qYigOWMYYe/HEWbBShY5ukhkTvcDxSfA4oJeyB/NAVjVXCi59HMSVG0Em ltlhA5ApgqM8mB9yZg00yB+1GbeopqjKCZSQbE+UDVrZ/SyWt6jKU7IwO0H3+sDaFqPj jvTg== 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=60wPXQppWk8s475AbmBzw6IxHES+gXNHfdpR+hDaVkc=; b=q3V2w37h5Y+y89CKqhRDPutx7RoJhbsGyL+PnppoQdB6YULU26YWUKR+d8ZedjWSrZ F34jtGHN4f5na0faUlekD5FsBgb6nU0hqe3oREDE/nMRuyq9gmbU/woaf634LbX052nt LYe6skLQFox1oxNe+RJ8feg8BK/ioHgTNCeo4UT3a5GldSrNgGLIup6w3wJUI6QVcXJv x7CY9vv/1JV4us//9fkp1q9CKyx+uPQXlmKIA7yFLu7EKLyIEFktKpsswAZOHMiPyzdI svcr7dE6muwFaOsOS0Me3KWa8xCz8iMBCyGGiMx4W8hz1ptPiABJsye7YUKzpkjjvhZP CMhA== X-Gm-Message-State: AOAM532oTKfeZTgOryj2kv7Y0/AkE6+40X6OICrGaBhtkqZssGrpy4uO HZ2TpAiJoDv7knZGG77tsrxkR0q0TxTIaWlt+jRyUDI+taJEhhW33hMrfWG1SkEAnxS96wocuQr PEqkQkrMNeiuYCN3w1nOblZDijXHdK/jUBHLPIpoQGnT+mBMoq6L7j6WkLRXyZncHKWZhxp6C2w kjTvUYciU3vcNa0ubg+q5itZtCPqLeT7B7iRwP4KhzxrcNT0N95lElq0DBNg== X-Google-Smtp-Source: ABdhPJzwvgUPfw5Iu3PVfbflNfb7Ce9RNL+suTbg+9a+138zY7g9zWm40M9BJiZB/6GpZ+/QYEMqfK45HvncWbK9QEw= X-Received: by 2002:a05:6e02:c71:: with SMTP id f17mr25255443ilj.176.1620720173666; Tue, 11 May 2021 01:02:53 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Tue, 11 May 2021 13:32:42 +0530 Message-ID: Subject: Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL To: Nikhil Mohite Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000003b730905c2095297" 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: Archived-At: Precedence: bulk --0000000000003b730905c2095297 Content-Type: text/plain; charset="UTF-8" Hi Nikhil Following are the review comments: *GUI specific*: - We need a panel icon for PSQL like query tool, we can also add that on the browser tree toolbar. - PSQL Tool menu should be visible for all the child nodes of the database node. Follow the same as Query Tool. - PSQL tab title should be only database server name as the user can change the database/user from PSQL command, so it's been difficult to update the tab title. - PSQL connection is still open even if we disconnect the database server from the browser tree. *Code specific:* - Remove an extra space from requirements.txt and package.json - Documentation needs to be updated to let the user know from where the PSQL tool will open and on which node it is applicable. - psql/__init__.py check there are so many unused imports please remove them. - We are not using cheroot so it should be removed from requirements.txt and also remove the import statement from pgAdmin4.py - Test cases are showing successful but actually, there are some routing errors please check. On Mon, May 10, 2021 at 7:32 PM Nikhil Mohite < nikhil.mohite@enterprisedb.com> wrote: > Hi Dave/ Team, > > PFA updated patch, sorry for the inconvenience, while cleanup I removed > the unwanted libraries but forgot to remove the code related to them. > > On Mon, May 10, 2021 at 7:10 PM Dave Page wrote: > >> Hi >> >> On Mon, May 10, 2021 at 1:45 PM Nikhil Mohite < >> nikhil.mohite@enterprisedb.com> wrote: >> >>> Hi Hackers, >>> >>> Please find the attached patch for RM-2341 >>> : Add Menu option for >>> starting PSQL. >>> 1. Added new Option PSQL Tool in Tools menu. >>> 2. Added the same option for Server and Database nodes from the tree >>> view. >>> >> >> Unfortunately there's a trailing comma in package.json that makes it >> invalid. If I fix that, then I get the error below, so I'm guessing the >> intention was to actually include another package there? >> >> ERROR in ./pgadmin/tools/psql/static/js/psql_module.js 23:50-82 >> Module not found: Error: Can't resolve 'local-echo-controller' in >> '/Users/dpage/git/pgadmin4/web/pgadmin/tools/psql/static/js' >> resolve 'local-echo-controller' in >> '/Users/dpage/git/pgadmin4/web/pgadmin/tools/psql/static/js' >> Parsed request is a module >> using description file: /Users/dpage/git/pgadmin4/web/package.json >> (relative path: ./pgadmin/tools/psql/static/js) >> aliased with mapping 'local-echo-controller': >> '/Users/dpage/git/pgadmin4/web/node_modules/local-echo' to >> '/Users/dpage/git/pgadmin4/web/node_modules/local-echo' >> using description file: /Users/dpage/git/pgadmin4/web/package.json >> (relative path: ./pgadmin/tools/psql/static/js) >> Field 'browser' doesn't contain a valid alias configuration >> root path /Users/dpage/git/pgadmin4/web >> using description file: >> /Users/dpage/git/pgadmin4/web/package.json (relative path: >> ./Users/dpage/git/pgadmin4/web/node_modules/local-echo) >> no extension >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo >> doesn't exist >> .js >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo.js >> doesn't exist >> .jsx >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo.jsx >> doesn't exist >> as directory >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo >> doesn't exist >> using description file: >> /Users/dpage/git/pgadmin4/web/package.json (relative path: >> ./node_modules/local-echo) >> no extension >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't >> exist >> .js >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo.js >> doesn't exist >> .jsx >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo.jsx >> doesn't exist >> as directory >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't >> exist >> @ ./pgadmin/tools/psql/static/js/index.js 17:19-43 >> >> 2021-05-10 14:38:37: webpack 5.21.2 compiled with 1 error in 60041 ms >> >> -- >> Dave Page >> Blog: https://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EDB: https://www.enterprisedb.com >> > > Regards, > Nikhil Mohite > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres * *Mobile: +91 976-788-8246* --0000000000003b730905c2095297 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Nikhil=C2=A0

Following are the revie= w comments:

GUI specific:
    We need a panel icon for PSQL like query tool, we can also add that on the= browser tree toolbar.
  • PSQL Tool menu should be visible for all the= child nodes of the database node. Follow the same as Query Tool.
  • P= SQL tab title should be only database server name as the user can change th= e database/user from PSQL command, so it's been difficult to update the= tab title.
  • PSQL connection is still open even if we disconnect the= database server from the browser tree.
Code specifi= c:
  • Remove an extra space from requirements.txt and pa= ckage.json
  • Documentation needs to be updated to let the user know f= rom where the PSQL tool will open and on which node it is applicable.
    • <= li>psql/__init__.py check there are so many unused imports please remove th= em.
  • We are not using cheroot so it should be removed from requireme= nts.txt and also remove the import statement from pgAdmin4.py=C2=A0
  • Test cases are showing=C2=A0successful but actually, there are some ro= uting errors please check.

On Mon, May 10, 2021 at 7:32 PM N= ikhil Mohite <nikhil.m= ohite@enterprisedb.com> wrote:
Hi Dave/ Team,

PFA updated patch, sorry for the inconvenience, while cleanup I re= moved the unwanted libraries but forgot to remove the code related to them.=

= On Mon, May 10, 2021 at 7:10 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, May 10, 2021 at 1:45 PM Nikhil Mohite <nikhil.mohi= te@enterprisedb.com> wrote:
Hi Hackers,

Please f= ind the attached patch for RM-2341:=C2=A0Add Menu option for starting PSQ= L.
1. Added new Option PSQL Tool in Tools menu.
2. Adde= d the same option for Server and Database nodes from the tree view.

Unfortunately=C2=A0there's a trail= ing comma in package.json that makes it invalid. If I fix that, then I get = the error below, so I'm guessing the intention was to actually include = another package there?

ERROR in ./pgadmin/too= ls/psql/static/js/psql_module.js 23:50-82
Module not found: Error= : Can't resolve 'local-echo-controller' in '/Users/dpage/gi= t/pgadmin4/web/pgadmin/tools/psql/static/js'
resolve 'loc= al-echo-controller' in '/Users/dpage/git/pgadmin4/web/pgadmin/tools= /psql/static/js'
=C2=A0 Parsed request is a module
= =C2=A0 using description file: /Users/dpage/git/pgadmin4/web/package.json (= relative path: ./pgadmin/tools/psql/static/js)
=C2=A0 =C2=A0 alia= sed with mapping 'local-echo-controller': '/Users/dpage/git/pga= dmin4/web/node_modules/local-echo' to '/Users/dpage/git/pgadmin4/we= b/node_modules/local-echo'
=C2=A0 =C2=A0 =C2=A0 using descrip= tion file: /Users/dpage/git/pgadmin4/web/package.json (relative path: ./pga= dmin/tools/psql/static/js)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 Field '= ;browser' doesn't contain a valid alias configuration
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 root path /Users/dpage/git/pgadmin4/web
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 using description file: /Users/dpage/git= /pgadmin4/web/package.json (relative path: ./Users/dpage/git/pgadmin4/web/n= ode_modules/local-echo)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= no extension
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fi= eld 'browser' doesn't contain a valid alias configuration
=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /Users/dpage/git/pgad= min4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't e= xist
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .js
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Field 'browser' doesn= 't contain a valid alias configuration
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 /Users/dpage/git/pgadmin4/web/Users/dpage/git/p= gadmin4/web/node_modules/local-echo.js doesn't exist
=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .jsx
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 Field 'browser' doesn't contain a vali= d alias configuration
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modu= les/local-echo.jsx doesn't exist
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 as directory
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_= modules/local-echo doesn't exist
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = using description file: /Users/dpage/git/pgadmin4/web/package.json (relativ= e path: ./node_modules/local-echo)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 no extension
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fie= ld 'browser' doesn't contain a valid alias configuration
<= div>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /Users/dpage/git/pgadmin4/web= /node_modules/local-echo doesn't exist
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 .js
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Field= 'browser' doesn't contain a valid alias configuration
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /Users/dpage/git/pgadmin4/web/n= ode_modules/local-echo.js doesn't exist
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 .jsx
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Fiel= d 'browser' doesn't contain a valid alias configuration
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /Users/dpage/git/pgadmin4/web/= node_modules/local-echo.jsx doesn't exist
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 as directory
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 /Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't e= xist
=C2=A0@ ./pgadmin/tools/psql/static/js/index.js 17:19-43

2021-05-10 14:38:37: webpack 5.21.2 compiled with 1 e= rror in 60041 ms
=C2=A0
--
=

Regards,
Nikhil Mohite=C2=A0


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

--0000000000003b730905c2095297--