public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dave Page <[email protected]>
To: Nikhil Mohite <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Akshay Joshi <[email protected]>
Subject: Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL
Date: Thu, 10 Jun 2021 12:52:10 +0100
Message-ID: <CA+OCxoypWKk=5uST1gQEyYpYfgiX4=_EVSpy6o+AF8LErMd3pA@mail.gmail.com> (raw)
In-Reply-To: <CAOBg0AMqZwQZoaxNHFn2cL0yHtfW04=RDYw-UqvjG-vA4KHmBA@mail.gmail.com>
References: <CAOBg0AO6Tjksb+EOA_-O13yRzdL_2d3y6G74m=GGQS+Ymjv=0g@mail.gmail.com>
<CA+OCxow9+UDMrsBBvJKW2AeAYaN4cO3w7kBGNKtsWhSM6_D7gQ@mail.gmail.com>
<CAOBg0AMVyLBma8wsbZ-VQWF4q3OAhqWeRLpmrxVcM_YOKH+MDQ@mail.gmail.com>
<CANxoLDd2xMu7ym9PFdhYUSy4ucNuPoZ2fvaRkuwyB+EqowLXwA@mail.gmail.com>
<CA+OCxowoz1oFE=eMKEnfFtf=oDrkPKHGfZpUGRW-sG_tpfMBTw@mail.gmail.com>
<CAOBg0ANFNvtD-LoFxXcLY5JJ4BVH5zNyrmdBED6B++-nKf012g@mail.gmail.com>
<CA+OCxowW7BEZzULbEpn73ygqWtWkUwdCAB3PMcHocUeQXcM3ow@mail.gmail.com>
<CAOBg0APGPTA4qNNryAXMwKP63uFLxQdQwnFdDXMjSr4OeH-HAg@mail.gmail.com>
<CA+OCxozYus58i3BhP37E-2Xc6Nq5zrJegcq+eANWpvN++_LtXw@mail.gmail.com>
<CAOBg0AO7NfJjLLFD8g5NkkbbnKiCSR5g9yshxc-65sWk2sFyBA@mail.gmail.com>
<CA+OCxoxdaBmh6e4Fo1F_+vZExq8SPUyGAByRzOPy2kz_vgRiSA@mail.gmail.com>
<CAOBg0AO6A10OHiQAOOguagW1mNzfK1oVPHG9W+JSCkYjR+xQkA@mail.gmail.com>
<CA+OCxoxpyfJabnycSGmtUK1-QEndNfZV3q2d1xP-oftDn-pGDw@mail.gmail.com>
<CANxoLDexHsJQcrd6ZrkjCub+xj_Ke+dDp_Av5no84MOJH1uC_g@mail.gmail.com>
<CAOBg0AN6Gb9gp1F=jKeefAz6d0U2GSjC40qAJ8i7psor2KmQLA@mail.gmail.com>
<CANxoLDd3QdOPY7Y6kDCSy99s3DFrbdhP9LNxphsEg9B5G14Dbg@mail.gmail.com>
<CAOBg0AN3eSLvS_cG7R3WiTpfBxiJfi2YB6sV8NBWiakioXRAsQ@mail.gmail.com>
<CANxoLDdGeNOBZJu-v9YaefKeTSGcCcU2DQic1MQU9jYYLJ6K_Q@mail.gmail.com>
<CAOBg0AO-gsDJSrLoGPJGKsq772Okgg3BWDuERaaGSDvoGbhQAA@mail.gmail.com>
<CA+OCxozEBo7-8hSeWo8K9mAL5E5MkeW3JG+JnjJXMEkT6q8SNg@mail.gmail.com>
<CANxoLDcfbqO5qYOV=LE8x1ocVEUoLD7bCm_UXYHmXL2=n9kX7A@mail.gmail.com>
<CAOBg0ANMJTzaEQdcEwUnLLGJXSbsT6=0L8Jvs_MTuBTfj0YtrQ@mail.gmail.com>
<CANxoLDcjno=sUmj11QSEv8OYCSoiOtMVvHjo=jrWjumBPtk6RA@mail.gmail.com>
<CAOBg0AMqZwQZoaxNHFn2cL0yHtfW04=RDYw-UqvjG-vA4KHmBA@mail.gmail.com>
Hi
On Thu, Jun 10, 2021 at 11:08 AM Nikhil Mohite <
[email protected]> wrote:
> Hi Dave/ Team,
>
> We are facing an issue with winpty.dll on Windows server 2016 and Windows
> 7(these are platforms on which we have tested).
> Files required for winpty are present in the site-packages but still, it
> is unable to load the winpty.dll file on these specific platforms. We have
> tested it on Windows 10 pro and Windows server 2019 and it is working fine.
> (Also tried building the local pywinpty but unable to build it.) ref link
> for winPty
> https://github.com/rprichard/winpty#:~:text=winpty%20is%20a%20Windows%20software,in%20a%20Cygwin%2FM....
>
Does winpty.exe run, if executed from the command line instead of pgAdmin?
If not, does dependency walker show any missing libraries that are required?
>
> Until we found the solution we can do the following solution for this.
> 1. We can add check if specific windows platform is not supporting winpty,
> show error msg when user open psql tool
> [image: image.png]
> 2. Diable the psql on these platforms by setting ENABLE_PSQL=False.
>
>
> If anyone has any other solution please let me know.
>
> Regards,
> Nikhil Mohite.
>
> On Tue, Jun 8, 2021 at 2:59 PM Akshay Joshi <[email protected]>
> wrote:
>
>> Thanks, the patch applied.
>>
>> On Mon, Jun 7, 2021 at 11:15 AM Nikhil Mohite <
>> [email protected]> wrote:
>>
>>> Hi Team,
>>>
>>> Please find the updated patch for added psql tool for windows platform.
>>> Also fixed a few issues reported by Fahar.
>>> 1. If the database name contains escape characters psql unable to
>>> connect.
>>> 2. If the user terminates the connection by entering "exit", psql will
>>> show connection termination msg.
>>>
>>>
>>> Regards,
>>> Nikhil Mohite.
>>>
>>> On Thu, Jun 3, 2021 at 2:10 PM Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Jun 3, 2021 at 2:07 PM Dave Page <[email protected]> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Tue, Jun 1, 2021 at 12:58 PM Nikhil Mohite <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Team,
>>>>>>
>>>>>> Following are few points related to PSQL tool on windows:
>>>>>>
>>>>>> 1. Currently using the *pywinpty* library on windows to create
>>>>>> pty process and execute the psql.exe.
>>>>>> 2. To read the stderr (errors) currently using '2>>&1' arguments
>>>>>> to psql.exe command. (It will redirect stderr to stdout )
>>>>>> 3. Windows conPTY is available on Windows 10 only (released after
>>>>>> 2018).
>>>>>> 4. Windows conPTY does not support the Asynchronous I/O, so to
>>>>>> get the terminal output, need to add the read function after every command
>>>>>> execution. (something like select() is not available)
>>>>>> 5. Also found some performance issues with psql on windows.
>>>>>> 1. To read the output from the terminal need to add some sleep
>>>>>> time as it will take time to return the output.
>>>>>> 2. Resize the terminal is also not consistent and causing the
>>>>>> issue if we resize the window faster or multiple times very quickly.
>>>>>> 3. Loading large dataset sometimes cause system to
>>>>>> non-responsive state.(In this state restart requires)
>>>>>>
>>>>>> Please find the patch for disable the psql tool for windows
>>>>>> platform.(Windows builds are falling due to this sending patch for disable
>>>>>> psql on windows.)
>>>>>>
>>>>>
>>>>> Disabling major features on our most common deployment platform really
>>>>> isn't a good option. I assume all of the above options are related to lack
>>>>> of async I/O?
>>>>>
>>>>
>>>> We have temporarily disabled the feature until found a solution, to
>>>> generate the nightly build for testing.
>>>>
>>>>>
>>>>> Have you tried forcing the use of winpty rather than conpty?
>>>>>
>>>>>
>>>>>>
>>>>>> Reference links:
>>>>>>
>>>>>> 1. https://github.com/microsoft/terminal/issues/262
>>>>>> 2.
>>>>>> https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-conso....
>>>>>> 3. https://pypi.org/project/pywinpty/
>>>>>>
>>>>>>
>>>>>> If any suggestions or questions please let me know.
>>>>>>
>>>>>> Regards,
>>>>>> Nikhil Mohite.
>>>>>>
>>>>>> On Tue, May 25, 2021 at 8:20 PM Akshay Joshi <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Thanks, patch applied.
>>>>>>>
>>>>>>> I have updated the screenshot and some documentation stuff.
>>>>>>>
>>>>>>> On Tue, May 25, 2021 at 3:08 PM Nikhil Mohite <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi Akshay,
>>>>>>>>
>>>>>>>>
>>>>>>>> Please find the updated patch. (V6)
>>>>>>>> On Tue, May 25, 2021 at 2:55 PM Akshay Joshi <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>> Hi Nikhil
>>>>>>>>>
>>>>>>>>> Please rebase and send the patch again.
>>>>>>>>>
>>>>>>>>> On Tue, May 25, 2021 at 2:52 PM Nikhil Mohite <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Akshay/ Team
>>>>>>>>>>
>>>>>>>>>> On Mon, May 24, 2021 at 9:19 PM Akshay Joshi <
>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Nikhil
>>>>>>>>>>>
>>>>>>>>>>> Following are the review comments:
>>>>>>>>>>>
>>>>>>>>>>> - Set "ENABLE_PSQL = False", PSQL button from browser tree
>>>>>>>>>>> and context menu option should not be visible.
>>>>>>>>>>> - Documentation screenshot should be in standard theme for
>>>>>>>>>>> consistency, and check the size it's very large. Take the screenshot with
>>>>>>>>>>> the new PSQL button on the browser tree.
>>>>>>>>>>> - Update 'menu_bar.rst' and 'toolbar.rst' with new changes.
>>>>>>>>>>> - Remove commented code (if any)
>>>>>>>>>>> - Check SonarQube (I haven't run)
>>>>>>>>>>>
>>>>>>>>>>> Please find the updated patch, resolve all the review comments,
>>>>>>>>>> and update the code to resolve the SonarQube issues.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Thu, May 20, 2021 at 2:52 PM Dave Page <[email protected]>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Thanks Nikhil. Can someone else review this version please?
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, May 19, 2021 at 2:42 PM Nikhil Mohite <
>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Dave/ Team,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, May 19, 2021 at 1:43 PM Dave Page <[email protected]>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, May 19, 2021 at 8:58 AM Nikhil Mohite <
>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Dave/ Team,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Tue, May 18, 2021 at 8:41 PM Dave Page <[email protected]>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Tue, May 18, 2021 at 12:12 PM Nikhil Mohite <
>>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Dave/Team,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Mon, May 17, 2021 at 6:47 PM Dave Page <
>>>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Mon, May 17, 2021 at 11:01 AM Nikhil Mohite <
>>>>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Akshay/ Team,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Please find the attached updated patch for the psql tool.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hmm, this version is also broken. There's a typo in
>>>>>>>>>>>>>>>>> editor_template.html on line 138 - it splits a string across two lines
>>>>>>>>>>>>>>>>> which throws an error. Having fixed that...
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I also note there's a lot of Javascript in that HTML file.
>>>>>>>>>>>>>>>>> That should be pushed into the webpacked bundle I think, and not included
>>>>>>>>>>>>>>>>> inline in HTML.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I have moved most of the code in the js file, few things
>>>>>>>>>>>>>>>> are still in HTML.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hmm, yes - in particular, colours for the different themes.
>>>>>>>>>>>>>>> Please move them into the css for the themes. You have a mix of style,
>>>>>>>>>>>>>>> layout and code in this file which needs to be cleaned up.
>>>>>>>>>>>>>>> xterm V3 onwards they have provided the API to set the theme
>>>>>>>>>>>>>>> and other settings, earlier I tried with CSS to override the theme but
>>>>>>>>>>>>>>> couldn’t able to apply the theme properly as some style get applied as
>>>>>>>>>>>>>>> in-line style for the HTML, so used the API to set the theme.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> OK, but either way we can't hard-code styles from themes in
>>>>>>>>>>>>>> HTML templates for individual features; that way leads to madness.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Perhaps Aditya or one of the other team members can give some
>>>>>>>>>>>>>> assistance?
>>>>>>>>>>>>>>
>>>>>>>>>>>>> I have moved the color settings to the respective theme files.
>>>>>>>>>>>>> Aditya helped in this.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Speaking of themes, the background colour for selected text
>>>>>>>>>>>>>>>> doesn't seem right (it's barely visible) in the dark theme. Can you fix
>>>>>>>>>>>>>>>> that to match the colouring in the SQL text boxes please?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I tried the default selection color from SQL for the dark
>>>>>>>>>>>>> and standard themes but still, it was not readable so just updated the
>>>>>>>>>>>>> color code with another color as follows.
>>>>>>>>>>>>> 1. Dark Theme:
>>>>>>>>>>>>> [image: Screenshot 2021-05-19 at 6.29.43 PM.png]
>>>>>>>>>>>>> 2. High Contrast: (using default SQL selection color)
>>>>>>>>>>>>> [image: Screenshot 2021-05-19 at 6.59.52 PM.png]
>>>>>>>>>>>>> 3. Standard:
>>>>>>>>>>>>> [image: image.png]
>>>>>>>>>>>>> can we go with the colors or should we update it?
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> A couple of other things I noticed:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> - The button is enabled if the treeview has a Server
>>>>>>>>>>>>>>>>>> selected. It could be argued that the query tool should do the same
>>>>>>>>>>>>>>>>>> (defaulting to the maintenance database), however, that would be a separate
>>>>>>>>>>>>>>>>>> change, and psql should be consistent with the query tool.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It is now consistent with the query tool.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> - If I do a "select * from pg_class;" I still get:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> postgres=# select * from pg_class;
>>>>>>>>>>>>>>>>>> WARNING: terminal is not fully functional
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I am not able to reproduce the warning for the terminal (I
>>>>>>>>>>>>>>>>> am working on Catalina 10.15.7), I checked on browsers (chrome, firefox,
>>>>>>>>>>>>>>>>> Safari) and also checked on local nwjs runtime but still not able to
>>>>>>>>>>>>>>>>> reproduce the warning. but found one limitation:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It looks like that can be fixed by adding:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> env={'TERM': 'xterm'}
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Added this in the environment when opening the psql panel.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> to the subprocess.Popen() call.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I noticed while I was playing with that, that you are
>>>>>>>>>>>>>>>> passing the password as part of the connection string. As I've mentioned in
>>>>>>>>>>>>>>>> the past, that is absolutely not acceptable; it will expose the password to
>>>>>>>>>>>>>>>> all manner of tools (such as ps -ef). You *must* pass the password to psql
>>>>>>>>>>>>>>>> using the PGPASSWORD environment variable.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> if we try to load data from the table containing millions
>>>>>>>>>>>>>>>>> of records, UI gets very slow.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Removed the password from the connection string and added
>>>>>>>>>>>>> 'PGPASSWORD' in the environment.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Is xtermjs discarding the older buffer contents when it
>>>>>>>>>>>>>>>> fills up? Can you tell where the memory usage is?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I checked the psql memory consumption in terminal and
>>>>>>>>>>>>>>> pgAdmin psql tool memory consumption is the similar. Also tested the
>>>>>>>>>>>>>>> performance and query execution timing is also similar.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> OK, so there's probably not much we can do here.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> - I'm sure using \q in the previous version displayed a
>>>>>>>>>>>>>>>>>> message saying the session exited (the one on line 138 of
>>>>>>>>>>>>>>>>>> editor_template.html). It no longer seems to do so.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> In addition to the issue above, it looks like the \!
>>>>>>>>>>>>>>>> blocking may have lost it's ability to ignore quoted strings:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> pgweb=# select '\!';
>>>>>>>>>>>>>>>> ERROR: Shell commands are disabled in psql for security
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Fixed the issue now it is consistent with the psql terminal.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Tue, May 11, 2021 at 3:40 PM Dave Page <
>>>>>>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On Tue, May 11, 2021 at 9:02 AM Akshay Joshi <
>>>>>>>>>>>>>>>>>>>> [email protected]> 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 <
>>>>>>>>>>>>>>>>>>>>> [email protected]> 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 <
>>>>>>>>>>>>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On Mon, May 10, 2021 at 1:45 PM Nikhil Mohite <
>>>>>>>>>>>>>>>>>>>>>>> [email protected]> 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
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>> Nikhil Mohite
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>> Nikhil Mohite
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> <https://www.enterprisedb.com;
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> *Thanks & Regards,*
>>>>>>>>>>>>>>> *Nikhil Mohite*
>>>>>>>>>>>>>>> *Software Engineer.*
>>>>>>>>>>>>>>> *EDB Postgres* <https://www.enterprisedb.com/;
>>>>>>>>>>>>>>> *Mob.No: +91-7798364578.*
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Dave Page
>>>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Nikhil Mohite
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Dave Page
>>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>>
>>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Thanks & Regards*
>>>>>>>>>>> *Akshay Joshi*
>>>>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>>>
>>>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Thanks & Regards*
>>>>>>>>> *Akshay Joshi*
>>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>>
>>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *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
>>>>>
>>>>>
>>>>
>>>> --
>>>> *Thanks & Regards*
>>>> *Akshay Joshi*
>>>> *pgAdmin Hacker | Principal Software Architect*
>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>
>>>> *Mobile: +91 976-788-8246*
>>>>
>>>
>>
>> --
>> *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
Attachments:
[image/png] Screenshot 2021-05-19 at 6.29.43 PM.png (81.5K, 3-Screenshot%202021-05-19%20at%206.29.43%20PM.png)
download | view image
[image/png] Screenshot 2021-05-19 at 6.59.52 PM.png (76.3K, 4-Screenshot%202021-05-19%20at%206.59.52%20PM.png)
download | view image
[image/png] image.png (52.6K, 5-image.png)
download | view image
[image/png] image.png (36.9K, 6-image.png)
download | view image
view thread (54+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL
In-Reply-To: <CA+OCxoypWKk=5uST1gQEyYpYfgiX4=_EVSpy6o+AF8LErMd3pA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox