Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Nikhil Mohite <nikhil(dot)mohite(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL
Date: 2021-05-11 08:02:42
Message-ID: CANxoLDd2xMu7ym9PFdhYUSy4ucNuPoZ2fvaRkuwyB+EqowLXwA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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(dot)mohite(at)enterprisedb(dot)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 <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Mon, May 10, 2021 at 1:45 PM Nikhil Mohite <
>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please find the attached patch for RM-2341
>>> <https://redmine.postgresql.org/issues/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 <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-05-11 08:19:03 pgAdmin 4 commit: Added support for cache bust webpack chunk files. Fix
Previous Message Aditya Toshniwal 2021-05-11 07:03:22 [pgAdmin][RM5477] Cache bust webpack chunk files