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

Hi Akshay/ Team,

Please find the attached updated patch for the psql tool.

On Tue, May 11, 2021 at 3:40 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Tue, May 11, 2021 at 9:02 AM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> 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.
>>
>> A few other things I noticed:
>
> - I was prompted to enter a password. This should be passed in the
> environment to psql as it is for pg_dump etc.
> - There seems to be an issue with terminal compatibility (which I didn't
> have on my prototype):
>
> ml=# select * from pg_class;
> WARNING: terminal is not fully functional
> -[ RECORD 1 ]-------+----------------------------------------------
> oid | 79354
> relname | housing
> ...
>
> - The panel should honour the styleguide. I'm running in dark mode, and
> see a jet black background. I would expect to see the same
> background/foreground colours as the treeview.
> - I spotted at least one print() statement that shouldn't be there (debug
> output should go through the logger) - psql/__init__.py:235
> - This seems suspect - why would there be a password in a connection
> string we've built? And why would it be xxx?
>
> if 'password=xxx' in conn_attr:
> conn_attr = conn_attr.replace('password=xxx', '')
>
> - There's a thick white line at the bottom of the panel, where a
> horizontal scrollbar might be if there was one.
> - The trailing semi-colon should be removed from: "ERROR: Shell commands
> are disabled in psql for security;"
>
> Once we're happy with the patch in general, I'll do a string review before
> committing. In particular, I want to be sure the text in config.py is
> appropriately worded.
>
> This is shaping up nicely! Good work.
>
>
>>
>> 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*
>>
>
>
> --
> Dave Page
> Blog: https://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EDB: https://www.enterprisedb.com
>
> Regards,
Nikhil Mohite