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

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

Hi

On Thu, Jun 10, 2021 at 2:00 PM Nikhil Mohite <
nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> On Thu, Jun 10, 2021 at 5:22 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Thu, Jun 10, 2021 at 11:08 AM Nikhil Mohite <
>> nikhil(dot)mohite(at)enterprisedb(dot)com> 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%2FMSYS%20pty.
>>>
>>
>> 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?
>>
> Not found the winpty.exe in site packages, but as per the winpty documents
> "winpty-agent.exe" will start the process with a new, hidden console
> window. It is not showing any error while installing the package. I tried
> to use it outside the pgAdmin but still facing the same error "Exception in
> import winpty DLL load failed while importing winpty: The specified
> procedure could not be found."(created a separate python environment for
> this)
>
> 1. if try to run winpty-agent.exe from the command line, it shows the user
> entered inputs on the same terminal, not showing any errors.
> [image: image.png]
>
> 2. I found winpty.exe in the other installed app (Git command line:
> C:\Program Files (x86)\Git\usr\bin) if try to run it through the command
> line not getting any error, it is showing user entered inputs on the same
> terminal. (It is not related to pgAdmin but just tried to check winpty.exe
> throwing any error or not)
> [image: image.png]
>

Hmmm, that sounds oddly similar to an issue I had with Kerberos on Windows
when I was mucking around with that. I can't remember the exact details,
but as a test, does the problem go away if winpty.dll is copied into the
Windows system32 directory?

>
>
>
>>
>>>
>>> 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 <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Thanks, the patch applied.
>>>>
>>>> On Mon, Jun 7, 2021 at 11:15 AM Nikhil Mohite <
>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> 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 <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Jun 3, 2021 at 2:07 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Tue, Jun 1, 2021 at 12:58 PM Nikhil Mohite <
>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> 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-console-conpty/#:~:text=The%20new%20ConPTY%20API%20will,version%20of%20Windows%20supports%20ConPTY.
>>>>>>>> 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 <
>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Thanks, patch applied.
>>>>>>>>>
>>>>>>>>> I have updated the screenshot and some documentation stuff.
>>>>>>>>>
>>>>>>>>> On Tue, May 25, 2021 at 3:08 PM Nikhil Mohite <
>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Akshay,
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please find the updated patch. (V6)
>>>>>>>>>> On Tue, May 25, 2021 at 2:55 PM Akshay Joshi <
>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Nikhil
>>>>>>>>>>>
>>>>>>>>>>> Please rebase and send the patch again.
>>>>>>>>>>>
>>>>>>>>>>> On Tue, May 25, 2021 at 2:52 PM Nikhil Mohite <
>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Akshay/ Team
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, May 24, 2021 at 9:19 PM Akshay Joshi <
>>>>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> 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 <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks Nikhil. Can someone else review this version please?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, May 19, 2021 at 2:42 PM Nikhil Mohite <
>>>>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Dave/ Team,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Wed, May 19, 2021 at 1:43 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Wed, May 19, 2021 at 8:58 AM Nikhil Mohite <
>>>>>>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Dave/ Team,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Tue, May 18, 2021 at 8:41 PM Dave Page <
>>>>>>>>>>>>>>>>> dpage(at)pgadmin(dot)org> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Tue, May 18, 2021 at 12:12 PM Nikhil Mohite <
>>>>>>>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Dave/Team,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Mon, May 17, 2021 at 6:47 PM Dave Page <
>>>>>>>>>>>>>>>>>> dpage(at)pgadmin(dot)org> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Mon, May 17, 2021 at 11:01 AM Nikhil Mohite <
>>>>>>>>>>>>>>>>>>> nikhil(dot)mohite(at)enterprisedb(dot)com> 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 <
>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>>>> 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
>>
>> Regards,
> Nikhil Mohite
>

--
Dave Page
Blog: https://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: https://www.enterprisedb.com

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Nikhil Mohite 2021-06-10 13:22:43 Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL
Previous Message Nikhil Mohite 2021-06-10 13:00:16 Re: [pgAdmin][RM-2341]: Add menu option for starting PSQL