Re: [pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Ashesh Vashi]

On Tue, Jun 12, 2018 at 10:04 AM, Ashesh Vashi <
ashesh.va...@enterprisedb.com> wrote:

> On Tue, Jun 12, 2018 at 10:02 AM, Robert Eckhardt 
> wrote:
>
>>
>>
>> On Tue, Jun 12, 2018, 12:27 AM Ashesh Vashi <
>> ashesh.va...@enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> On Tue, Jun 12, 2018 at 1:50 AM, Joao De Almeida Pereira <
>>> jdealmeidapere...@pivotal.io> wrote:
>>>
 Hi Hackers,

 Attached you can find the diff file that corrects RM 3415
 

>>> If I understand correctly, you're disabling the privileges for GPDB.
>>> Is that correct?
>>>
>>
>> It should be only for Greenplum.
>>
> Yeah - I got that.
>
>
>> There was a performance optimization in Postgres 9.2 that make the
>> current query in Postgres performant.
>>
> Ok.
>
> Changes looks good to me.
>
Committed!

-- Thanks, Ashesh

>
> -- Thanks, Ashesh
>
>>
>> Rob
>>
>>
>>> -- Thanks, Ashesh
>>>
>>> ​
 Thanks
 Joao

>>>
>>>
>

pgAdmin 4 commit: Disable ACLs for the database objects for the Greenpl

[Ashesh Vashi]

Disable ACLs for the database objects for the Greenplum due to
performance issue.

Reviewed By: Ashesh
Fixes #3415

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=e5048b4d0b8fd9b2b556e33231d2a77d4f7b01df
Author: Joao De Almeida Pereira 

Modified Files
--
.../foreign_servers/sql/gpdb_5.0_plus/acl.sql  |  7 ++
.../sql/gpdb_5.0_plus/acl.sql  |  7 ++
.../templates/languages/sql/gpdb_5.0_plus/acl.sql  | 41 ++--
.../foreign_tables/sql/gpdb_5.0_plus/acl.sql   |  7 ++
.../templates/function/gpdb/sql/default/acl.sql| 42 ++--
.../templates/sequence/sql/gpdb_5.0_plus/acl.sql   |  7 ++
.../templates/column/sql/gpdb_5.0_plus/acl.sql |  7 ++
.../templates/table/sql/gpdb_5.0_plus/acl.sql  |  7 ++
.../templates/catalog/gpdb_5.0_plus/sql/acl.sql| 43 ++--
.../templates/schema/gpdb_5.0_plus/sql/acl.sql | 43 ++--
.../views/templates/view/gpdb_5.0_plus/sql/acl.sql | 78 ++
.../templates/databases/sql/gpdb_5.0_plus/acl.sql  |  7 ++
.../tablespaces/sql/gpdb_5.0_plus/acl.sql  |  7 ++
13 files changed, 86 insertions(+), 217 deletions(-)

Re: RM#3294 - User need to reset the layout to see the changed preferences parameters

[Aditya Toshniwal]

Hi Harshal,

On Mon, Jun 11, 2018 at 6:37 PM, Harshal Dhumal <
harshal.dhu...@enterprisedb.com> wrote:

> On Mon, Jun 11, 2018 at 6:02 PM, Aditya Toshniwal  enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> This is regarding RM3294 which I am working on, where user needs to reset
>> the layout to see the changed preferences parameters. I am able to
>> implement it, and have used the following approach:
>> 1) when preference changed, fire a custom event - prefChangedEvent
>>
> We can also fire preference specific event instead of generic event.
>
I have just sent the overall flow and not the detailed. I am going to make
it general event with data as modules whose preferences are changed.

> And we can generate event name from preference itself like
> *:::*
> eg.:* browser:display:show_system_objects:update*
>
This will require lot many if clauses and with so many preferences I think
we should do it at module level.

>
>
>> 2) listent to prefChangedEvent wherever preferences should be reflected
>> in realtime.
>> 3) In the event handler, make the changes as per the module.
>> 4) So whenever the preferences are changed, the event fires, the listener
>> executes the event handler and changes are done.
>>
>> Benefit of this approach is no polling. But, there is a problem here.
>> It is not possible to fire event in another tab/new browser window. For
>> example, query tool can be open in another tab. And thus, changes are not
>> reflected there. There are solutions available like updating the
>> localStorage of the browser but those are not reliable and does not work
>> properly on different browsers.
>>
> To communicate between browser tabs we can use cookie polling on client
> side it self (at least it will avoid polling over http).
> The main tab will update only preference specific cookie when preference
> is updated and other tabs will poll required cookies (not all)
> with specific interval (1 second can be configurable).
>
Polling is a solution but I think it should be the last option. http polls
will not be required anyway as we have preference cache in the browser
object.

>
>
>>
>> Request you to kindly suggest if you any better idea.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>
>

Re: [pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Ashesh Vashi]

On Tue, Jun 12, 2018 at 10:02 AM, Robert Eckhardt 
wrote:

>
>
> On Tue, Jun 12, 2018, 12:27 AM Ashesh Vashi 
> wrote:
>
>> Hi Joao,
>>
>> On Tue, Jun 12, 2018 at 1:50 AM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached you can find the diff file that corrects RM 3415
>>> 
>>>
>> If I understand correctly, you're disabling the privileges for GPDB.
>> Is that correct?
>>
>
> It should be only for Greenplum.
>
Yeah - I got that.


> There was a performance optimization in Postgres 9.2 that make the current
> query in Postgres performant.
>
Ok.

Changes looks good to me.

-- Thanks, Ashesh

>
> Rob
>
>
>> -- Thanks, Ashesh
>>
>> ​
>>> Thanks
>>> Joao
>>>
>>
>>

Re: [pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Robert Eckhardt]

On Tue, Jun 12, 2018, 12:27 AM Ashesh Vashi 
wrote:

> Hi Joao,
>
> On Tue, Jun 12, 2018 at 1:50 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Hackers,
>>
>> Attached you can find the diff file that corrects RM 3415
>> 
>>
> If I understand correctly, you're disabling the privileges for GPDB.
> Is that correct?
>

It should be only for Greenplum. There was a performance optimization in
Postgres 9.2 that make the current query in Postgres performant.

Rob


> -- Thanks, Ashesh
>
> ​
>> Thanks
>> Joao
>>
>
>

Re: [pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Ashesh Vashi]

Hi Joao,

On Tue, Jun 12, 2018 at 1:50 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Hackers,
>
> Attached you can find the diff file that corrects RM 3415
> 
>
If I understand correctly, you're disabling the privileges for GPDB.
Is that correct?

-- Thanks, Ashesh

​
> Thanks
> Joao
>

[pgadmin4][patch][GreenPlum] Display SQL for tables takes 15 minutes to execute

[Joao De Almeida Pereira]

Hi Hackers,

Attached you can find the diff file that corrects RM 3415

​
Thanks
Joao


RM_3415.diff
Description: Binary data

Re: [PATCH] pgAgent segfault

[Dave Page]

Thanks, patch applied.

On Mon, Jun 11, 2018 at 5:25 AM, Ashesh Vashi  wrote:

> Hi Dave,
>
> While debugging the issue
> 
> reported by Rob, I found one segfault.
> Please find the attached patch to find that fix along with another one.
>
> 1. Connection object was being accessed, which was already released.
>
> 2. Changed the logic for checking the connection object existence.
>
> We're using "if (!ms_primaryCon)", which should convert the logic as
> boolean operator explicitly, which we defined for the 'DBconn' class.
>
> I suspect some compilers (specifically windows one) may not behave that
> way, and instead of using the boolean operator, it checks for the pointer
> is NULL or not.
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>



-- 
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

pgAgent commit: 1. Connection object was being accessed, which was alre

[Dave Page]

1. Connection object was being accessed, which was already released.

2. Changed the logic for checking the connection object existence.

We're using "if (!ms_primaryCon)", which should convert the logic as boolean 
operator explicitly, which we defined for the 'DBconn' class.

I suspect some compilers (specifically windows one) may not behave that way, 
and instead of using the boolean operator, it checks for the pointer is NULL or 
not.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgagent.git;a=commitdiff;h=1c583df4bbf0a3f15a5f919096057cca70da1237
Author: Ashesh Vashi 

Modified Files
--
connection.cpp | 2 +-
pgAgent.cpp| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

Re: [pgAdmin4][RM#3404] Graphical explain plan do not display text under the node

[Dave Page]

Thanks, patch applied.

On Sat, Jun 9, 2018 at 7:01 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> Sorry, the previous patch had a typo, Here's an updated patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Thu, Jun 7, 2018 at 2:32 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Wed, Jun 6, 2018 at 6:04 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> PFA minor patch to fix the regression issue introduced with RM#3271
>>> commit due to which label is not displaying under the graphical explain
>>> node, I've attached the screenshot in RM for your reference.
>>>
>>
>> I see the labels, but the popups are not displaying on mouseover/hover
>> like they used to. Can you look into that please?
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Fix labels and popups on EXPLAIN plan that were broke

[Dave Page]

Fix labels and popups on EXPLAIN plan that were broken by the jQuery 3 update. 
Fixes #3404

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=e2b0cf6727c991727799d2588d41366aaf58633a
Author: Murtuza Zabuawala 

Modified Files
--
web/pgadmin/misc/static/explain/js/explain.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Re: [pgAdmin][patch] RM3409 - Retrieving SQL from table throws exception

[Dave Page]

Thanks, patch applied.

On Fri, Jun 8, 2018 at 7:43 PM, Victoria Henry  wrote:

> Hi hackers,
>
> Attached is a patch that fixes an exception when trying to view the SQL
> tab on a GreenPlum database.
>
> Sincerely,
>
> Joao && Victoria
>



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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgAdmin 4 commit: Avoid an exception on GreenPlum when retrieving RE-SQ

[Dave Page]

Avoid an exception on GreenPlum when retrieving RE-SQL on a table. Fixes #3409

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=226c872a848c2fe16a1d7d57a1b4141f92493e22
Author: Victoria Henry 

Modified Files
--
docs/en_US/release_notes_3_1.rst   |  3 ++-
.../templates/trigger/sql/9.1_plus/properties.sql  | 23 +
.../templates/trigger/sql/default/properties.sql   | 24 +-
.../servers/databases/schemas/tables/utils.py  |  2 ++
4 files changed, 28 insertions(+), 24 deletions(-)

Re: [pgadmin][patch] Electron version 4.X

[Dave Page]

Hi

On Fri, Jun 8, 2018 at 3:49 PM, Victoria Henry  wrote:

>
>
> On Tue, Jun 5, 2018 at 12:28 PM Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Jun 4, 2018 at 10:27 PM, Joao De Almeida Pereira <
>> jdealmeidapere...@pivotal.io> wrote:
>>
>>> Hi Hackers,
>>>
>>> Attached you can find the patch that introduces electron to our code
>>> base.
>>>
>> Cool. FYI, I'd like to get this into rough shape and then push it to a
>> dev branch for fine-tuning. I think it'll be easier to work that way.
>>
> Sure, you can create a dev branch and push this patch to it.
>
> Great. So here's my initial feedback:
>>
>> - The Mac build you provided doesn't work for me. It hangs on the loading
>> screen.
>>
> Could we get some more information about the machine?  Using the python
> from the venv directory, are you able to run the app directly?
>
> cd /Applications/pgAdmin.app/Contents/Resources/app/
> venv/bin/python web/pgAdmin4.py
>
> ​
> Do you get and error?
> We are setting PGADMIN_PORT, PGADMIN_KEY and SERVER_MODE environment
> variables prior to starting it.
>

Yes, I get:

piranha:app dpage$ venv/bin/python web/pgAdmin4.py

Could not find platform independent libraries 

Could not find platform dependent libraries 

Consider setting $PYTHONHOME to [:]

Fatal Python error: Py_Initialize: unable to load the file system codec

ModuleNotFoundError: No module named 'encodings'


Current thread 0x7fffe86993c0 (most recent call first):

Abort trap: 6




>
>
>>
>>
> - A number of the changes are related to the naming of requirejs. I'd be
>> inclined to pull that out into a separate patch and get it committed to
>> master ASAP.
>>
> This change only makes sense in the Electron context. Neverthless fell
> free to add it to master if you think it is relevant.
>
>
>> - I think the build instructions need to be more generic (particularly on
>> macOS). For example, I do not use HomeBrew (largely due to some nasty
>> security issues they had in the past). I was able to mostly port the
>> instructions and build script over to work using MacPorts (without PyEnv)
>> which actually turned out to be somewhat more simple than what's there now.
>>
> Since we don't use MacPorts, we cannot provide installation instructions.
>

I wasn't asking you to provide MacPort instructions; I can do that. I was
saying that the basic outline of the instructions should be agnostic to the
package tools the user might choose; e.g.

- Install python
- Create a virtual environment using the --copies option called venv
- Move the virtual environment to ...

We can then include examples of how to do that with brew, macports etc.


>
>
>> - I'm not sure what this is intended to do: "git checkout electron".
>> Clearly that isn't correct.
>>
> That was the name of our development branch.  It can be removed.
>

OK.


>
>
>>
>> - All new builds should be using Python 3.6. We need to deprecate 2.7 as
>> there are some Unicode related issues that cannot be fixed in it.
>>
> For Windows, we are using 2.7 because of external library compilation
> issues.   Let us know if you are able to get around this or how to make
> this work.
>

There are a couple of tricks if you're using Python 3.6. First, to compile
Pycrypto you need to do something like:


   - set VCINSTALLDIR=C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC
   - set CL=-FI"%VCINSTALLDIR%\INCLUDE\stdint.h"
   - (https://stackoverflow.com/questions/41843266/
   microsoft-windows-python-3-6-pycrypto-installation-error
   

   )

Beforehand. Then, edit Lib\site-packages\Crypto\Random\OSRNG\nt.py" and
modify " *import winrandom*" to "*from . import winrandom*".

As a sidenote, I use the VC++ 2015 Build Tools compiler from Microsoft,
without installing any SDKs or Visual Studio.


>
>
>> - I would like to see the new build code adapted to follow the existing
>> conventions as much as reasonable, to avoid having to change build systems
>> or other processes/procedures that folks use. For example, build scripts
>> should be under pkg/, completed packages left in dist/, build staging done
>> in xxx-build directories rather than elsewhere.
>>
> That sounds reasonable.
>
>
>> - It may be a result of my use of MacPorts, but I'm getting the following
>> failure building:
>>
>
>> yarn run v1.3.2
>> $ electron-forge make --platfrom=darwin --arch=x64 --targets=dmg
>> ✔ Checking your system
>> ✔ Resolving Forge Config
>> We need to package your application before we can make it
>> ✔ Preparing to Package Application for arch: x64
>> ⠼ Compiling ApplicationFailed to compile file:
>> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> Contents/Resources/app/venv/lib/python3.6/site-packages/setuptools/command/launcher
>> manifest.xml
>> Compiling /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-
>> packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/
>> 

Re: [pgadmin4][patch] Use pytest test runner for unit tests

[Dave Page]

Hi

On Mon, Jun 4, 2018 at 3:26 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Known issues:
>>>
>>>- Python 2.7, the library we are using for assertions (Grappa) is
>>>failing while trying to assert on strings. We created a PR to the 
>>> library:
>>>https://github.com/grappa-py/grappa/pull/43
>>> as soon as this gets
>>>in all the tests should pass
>>>
>>> Any guesses as to the ETA? Given that most of our dev, and our Windows
>> and Mac packages both run on 2.7 at the moment, it's clear that this is a
>> required fix before we can proceed.
>>
>
> Attached you can find the patch that bumps grappa to version 0.1.9 that
> support Python 2.7
>

I thought I had replied to this, but apparently not. My apologies...

This does seem to fix the issues with Python 2.7. The remaining issues I
saw are:

- There are a handful of failures with PG 11, however that's not yet
supported and Khushboo is working on it. It shouldn't hold up this patch -
just mentioning for completeness.

- I get the following failure (which is easily fixed in the config file,
though that of course, shouldn't be necessary):

E   AssertionError: Oops! Something went wrong!

E

E The following assertion was not satisfied

E   subject "/Library/PostgreSQL/ ..." should be equal to
"/Library/PostgreSQL/ ..."

E

E What we expected

E   a value that is equal to "/Library/PostgreSQL/ ..."

E

E What we got instead

E   an value of type "str" with data "/Library/PostgreSQL/ ..."

E

E Difference comparison

E   > - /Library/PostgreSQL/tablespaces/9.4

E   > + /Library/PostgreSQL/tablespaces/9.4/

E   > ?+

E

E Where

E   File
"/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/tablespaces/tests/test_tbspc_get.py",
line 75, in test_tablespace_get

Thanks.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: RM#3294 - User need to reset the layout to see the changed preferences parameters

[Harshal Dhumal]

On Mon, Jun 11, 2018 at 6:02 PM, Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Hackers,
>
> This is regarding RM3294 which I am working on, where user needs to reset
> the layout to see the changed preferences parameters. I am able to
> implement it, and have used the following approach:
> 1) when preference changed, fire a custom event - prefChangedEvent
>
We can also fire preference specific event instead of generic event.
And we can generate event name from preference itself like
*:::*
eg.:* browser:display:show_system_objects:update*


> 2) listent to prefChangedEvent wherever preferences should be reflected in
> realtime.
> 3) In the event handler, make the changes as per the module.
> 4) So whenever the preferences are changed, the event fires, the listener
> executes the event handler and changes are done.
>
> Benefit of this approach is no polling. But, there is a problem here.
> It is not possible to fire event in another tab/new browser window. For
> example, query tool can be open in another tab. And thus, changes are not
> reflected there. There are solutions available like updating the
> localStorage of the browser but those are not reliable and does not work
> properly on different browsers.
>
To communicate between browser tabs we can use cookie polling on client
side it self (at least it will avoid polling over http).
The main tab will update only preference specific cookie when preference is
updated and other tabs will poll required cookies (not all)
with specific interval (1 second can be configurable).


>
> Request you to kindly suggest if you any better idea.
>
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Ashesh Vashi]

On Mon, Jun 11, 2018 at 3:59 PM, Ashesh Vashi  wrote:

> Hi Team,
>
> On Sat, May 5, 2018 at 3:31 AM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> *...*
>> 3. Start the discussion on application architecture
>>
>> Why should we care about location of files inside a our application?
>>
>> Why is this way better the another?
>>
>> These are 2 good questions that have very lengthy answers. Trying to more
>> or less summarize the answers we care about
>> the location of the files, because we want our application to communicate
>> intent and there are always pros and cons
>> on all the decisions that we make.
>>
> To be honest, it depends on how do you see the application, and its
> expectations.
> That question leads me to another question "What do we want from pgAdmin
> 4?"
>
> At this point the application structure follows our menu, this approach
>> eventually make is easier to follow the code
>> but at the same time if the menu changes down the line, will we change
>> the structure of our folders?
>>
> To be honest not menu driven. (Probably a wrong choice of word 'Menu'.)
> But - only under 'browser' (You can also call it object browser/tree), it
> is driven by the database object relation model.
> And, each module maintains the parent-child relationship.
>
>
>> The proposal that we do with the last diff of this patch is to change to
>> a structure that slices vertically the
>> application. This way we can understand intent behind the code and more
>> easily find what we are looking for.
>>
> In the current structure if you want to see the tables code you need to go
>> to
>> pgAdmin/browser/server_groups/servers/databases/schemas/tables/ this is
>> a huge path to remember and to get to. What
>> do we win with this?
>>
> I agree - it is too deep structure.
> But - it gives me the idea what's the structure of the database objection
> relationship.
>
> Does it hurt, I would say no?
> Because - as a database developer, I know whats the relationship of
> objects, and where I can find them.
> (Until I heard Dave saying it is about to touch the limit of maximum file
> system path.)
>
>
So - there is a scope for improvement there, we can leave *live* without
the object relations (but - limited to the object browser I think).


>
>
>> If we open pgAdmin we know which nodes to click in order to get to
>> tables. But for development
>> every time that you are looking for a specific functionality you need to
>> run the application, navigate the menu so that
>> you know where you can find the code. This doesn’t sound very appealing.
>>
> What if our structure would look like this:
>>
>>  - web
>>- tables
>>  - controller
>>- get_nodes.py
>>- get_sql.py
>>- __init__.py
>>  - frontend
>> - component
>> - ddl_component.js
>> - services
>> - table-service.js
>>- schemas
>>- servers
>>- 
>>
>> I think - there is nothing wrong with the current module structure. It is
> more appealing to me than the above one.
> The python package contains the backend code, and static contains all your
> frontend.
>
> I am not saying, we should not change anything.
> We can definitely divide them in smaller chunks for both backend, and
> frontend side.
>
> This would saves us time because all the information that we need is what
>> are we working on and everything is right there.
>> Menu driven structure Intent Driven Structure
>> *Pros:* *Pros:*
>> Already in place Explicitly shows features
>> Self contained features Self contained features
>> Support for drop in features Support for drop in features
>> *Cons:* *Cons:*
>> Follows the menu, and it could change Need to change current code
>> Hard to find features Some additional plumbing might be needed
>> Drop in features need to be placed in a specific location according to
>> the menu location
>>
>> What are your thought about this architecture?
>>
> I am not a fan of flat file structure in application.
> There are many reasons - why we need namespace in C++, same applies here.
>
> Let me start from the question "What do we want from pgAdmin 4?"
> * A object browser
> * Miscallenous operations associated with the object shown in the object
> browser
>   + Reversed Enginering query (if any)
>   + Properties Viewer
>   + Edit/Create Viewer
>   + Staticis
>   + List of dependencies
>   + List of dependents
> * Query tool
> * Data Viewer (table/view)
> * Function debugger
> * Extendability/Pluggability
>
> According to me - these as the intent of the application, and not objects
> assiciated with them.
>
> I do agree, there is no boundry (or, very thin) between data &
> presentation at the moment.
> We can start from there.
>
> I can think of the following directory structure ( which is not too
> different from the current, but - still will give the developers a lot more
> comfort as per your complain :-) ).
>
>> pgadmin/
>> - database_objects/
>>   - server/
>> - 

Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

[Ashesh Vashi]

Hi Team,

On Sat, May 5, 2018 at 3:31 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> *...*
> 3. Start the discussion on application architecture
>
> Why should we care about location of files inside a our application?
>
> Why is this way better the another?
>
> These are 2 good questions that have very lengthy answers. Trying to more
> or less summarize the answers we care about
> the location of the files, because we want our application to communicate
> intent and there are always pros and cons
> on all the decisions that we make.
>
To be honest, it depends on how do you see the application, and its
expectations.
That question leads me to another question "What do we want from pgAdmin 4?"

At this point the application structure follows our menu, this approach
> eventually make is easier to follow the code
> but at the same time if the menu changes down the line, will we change the
> structure of our folders?
>
To be honest not menu driven. (Probably a wrong choice of word 'Menu'.)
But - only under 'browser' (You can also call it object browser/tree), it
is driven by the database object relation model.
And, each module maintains the parent-child relationship.


> The proposal that we do with the last diff of this patch is to change to a
> structure that slices vertically the
> application. This way we can understand intent behind the code and more
> easily find what we are looking for.
>
In the current structure if you want to see the tables code you need to go
> to
> pgAdmin/browser/server_groups/servers/databases/schemas/tables/ this is a
> huge path to remember and to get to. What
> do we win with this?
>
I agree - it is too deep structure.
But - it gives me the idea what's the structure of the database objection
relationship.

Does it hurt, I would say no?
Because - as a database developer, I know whats the relationship of
objects, and where I can find them.
(Until I heard Dave saying it is about to touch the limit of maximum file
system path.)

So - there is a scope for improvement there, we can leave without the
object relations (but - limited to the object browser I think).


> If we open pgAdmin we know which nodes to click in order to get to tables.
> But for development
> every time that you are looking for a specific functionality you need to
> run the application, navigate the menu so that
> you know where you can find the code. This doesn’t sound very appealing.
>
What if our structure would look like this:
>
>  - web
>- tables
>  - controller
>- get_nodes.py
>- get_sql.py
>- __init__.py
>  - frontend
> - component
> - ddl_component.js
> - services
> - table-service.js
>- schemas
>- servers
>- 
>
> I think - there is nothing wrong with the current module structure. It is
more appealing to me than the above one.
The python package contains the backend code, and static contains all your
frontend.

I am not saying, we should not change anything.
We can definitely divide them in smaller chunks for both backend, and
frontend side.

This would saves us time because all the information that we need is what
> are we working on and everything is right there.
> Menu driven structure Intent Driven Structure
> *Pros:* *Pros:*
> Already in place Explicitly shows features
> Self contained features Self contained features
> Support for drop in features Support for drop in features
> *Cons:* *Cons:*
> Follows the menu, and it could change Need to change current code
> Hard to find features Some additional plumbing might be needed
> Drop in features need to be placed in a specific location according to the
> menu location
>
> What are your thought about this architecture?
>
I am not a fan of flat file structure in application.
There are many reasons - why we need namespace in C++, same applies here.

Let me start from the question "What do we want from pgAdmin 4?"
* A object browser
* Miscallenous operations associated with the object shown in the object
browser
  + Reversed Enginering query (if any)
  + Properties Viewer
  + Edit/Create Viewer
  + Staticis
  + List of dependencies
  + List of dependents
* Query tool
* Data Viewer (table/view)
* Function debugger
* Extendability/Pluggability

According to me - these as the intent of the application, and not objects
assiciated with them.

I do agree, there is no boundry (or, very thin) between data & presentation
at the moment.
We can start from there.

I can think of the following directory structure ( which is not too
different from the current, but - still will give the developers a lot more
comfort as per your complain :-) ).

> pgadmin/
> - database_objects/
>   - server/
> - __init__.py
> - create.py
> - get.py
> - sql.py
> - update.py
> - static/
>   - img/
> - server.svg
> - server_bad.svg
>   - javascripts/
> - server.js
> - ui_schema.js
>   - database/
>   - templates

Re: PGAgent 4.0 error feedback

[Robert Emery]

Hello Again,

I've just tested the patch and the output is *much* more verbose with the same
set of flags now, even more so than the old version (see attached).

Thanks,
Rob

On 11/06/2018, Ashesh Vashi  wrote:
> Hi Rob,
>
>
> On Mon, Jun 11, 2018 at 12:50 PM, Rob Emery 
> wrote:
>
>> Hi Ashesh,
>>
>> Log-level wise I was using the same command for both versions:
>>
>>  ./pgagent -f -l 2 -t 1 -r 1 'hostaddr=server user=username
>> password=password dbname=postgres'
>>
>> and messing up the connection string in various ways (missing dbname,
>> incorrect password, incorrect hostaddr)
>>
>> This was built and tested on  Debian GNU/Linux 9.4 (stretch).
>>
> If you're working on the source, would please test the patch sent by me?
>
>>
>> I've attached an example of the differing output when the dbname is
>> missing
>> from the connectionstring; basically it's the same for all errors.
>>
>
> Thanks a lot for the information.
>
> -- Thanks, Ashesh
>
>>
>> Many Thanks,
>> Rob
>>
>> On 11/06/2018, Ashesh Vashi  wrote:
>> > Hi Rob,
>> >
>> > On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery 
>> > wrote:
>> >
>> >> Hi Guys,
>> >>
>> >> I've been testing out PGAgent 4 (build today from master at commit
>> >> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
>> >> handling on error is still fixed with the boost reimplementation
>> >> (which it is!)
>> >> and noticed a few issues compared to PGAgent 3 around feedback to the
>> >> user if the connection string is incorrect.
>> >>
>> >> I've tested with incorrect username and password, user not in the hba,
>> >> db doesn't exist, postgresql not running on the host (or incorrect
>> >> hostaddr) and 4 seems to always returns the error :
>> >> `
>> >> ERROR: Couldn't find the function 'pgagent_schema_version' - please
>> >> run pgagent_upgrade.sql.
>> >> `
>> >>
>> >> whereas 3.4.1 tends to output from the pgconnection itself like:
>> >> `
>> >> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
>> >> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
>> >> "172.30.0.16", user "pgagent_login_role", database
>> >> "pgagent_login_role", SSL on
>> >> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
>> >> "pgagent_login_role", database "pgagent_login_role", SSL off
>> >> `
>> >>
>> >> which at least points the user towards the actual error!
>> >>
>> >> Not sure if this is known already, but I thought I'd raise it prior to
>> >> release.
>> >>
>> > Thanks for sharing the information.
>> >
>> > Is it possible for you to share the logs with log level set to debug?
>> > Also - please share the operating system details?
>> >
>> > --
>> >
>> > Thanks & Regards,
>> >
>> > Ashesh Vashi
>> > EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> > 
>> >
>> >
>> > *http://www.linkedin.com/in/asheshvashi*
>> > 
>> >
>> >
>> >
>> >>
>> >> Thanks,
>> >> Rob
>> >> --
>> >>
>> >> --
>> >>  
>> >>
>> >>
>> >> Codeweavers May Newsletter
>> >>   l  Codeweavers
>> >> April
>> >> Finance Trends
>> >> 
>> >>
>> >>
>> >> April’s Dealer Highlights
>> >> 
>> >> > march-car-finance-trends
>> >> >
>> >>
>> >> _
>> >> _
>> >>
>> >>
>> >>
>> >> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> >> 
>> >> Codeweavers Ltd | Barn 4 | Dunston
>> >> Business Village | Dunston | ST18 9AB
>> >> Registered in England and Wales No.
>> >> 04092394 | VAT registration no. 974 9705 63
>> >>
>> >>
>> >>
>> >>
>> >> 
>> >> 
>> >> 
>> >> 
>> >>
>> >>
>> >>
>> >
>>
>> --
>>  
>>
>>
>> Codeweavers May Newsletter
>>   l  Codeweavers
>> April
>> Finance Trends
>> 
>>
>>
>> April’s Dealer Highlights
>> 
>> > >
>>
>> _
>> _
>>
>>
>>
>> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> 
>> Codeweavers Ltd | Barn 4 | Dunston
>> Business Village | Dunston | ST18 9AB
>> Registered in England and Wales No.
>> 04092394 | VAT registration no. 974 9705 63
>>
>>
>>
>>
>> 
>> 
>> 
>> 
>>
>>
>


-- 
Robert Emery
Infrastructure 

Fwd: PGAgent 4.0 error feedback

[Rob Emery]

Hello Again,

I've just tested the patch and the output is *much* more verbose with the same
set of flags now, even more so than the old version (see attached).

Thanks,
Rob

On 11/06/2018, Ashesh Vashi  wrote:
> Hi Rob,
>
>
> On Mon, Jun 11, 2018 at 12:50 PM, Rob Emery 
> wrote:
>
>> Hi Ashesh,
>>
>> Log-level wise I was using the same command for both versions:
>>
>>  ./pgagent -f -l 2 -t 1 -r 1 'hostaddr=server user=username
>> password=password dbname=postgres'
>>
>> and messing up the connection string in various ways (missing dbname,
>> incorrect password, incorrect hostaddr)
>>
>> This was built and tested on  Debian GNU/Linux 9.4 (stretch).
>>
> If you're working on the source, would please test the patch sent by me?
>
>>
>> I've attached an example of the differing output when the dbname is
>> missing
>> from the connectionstring; basically it's the same for all errors.
>>
>
> Thanks a lot for the information.
>
> -- Thanks, Ashesh
>
>>
>> Many Thanks,
>> Rob
>>
>> On 11/06/2018, Ashesh Vashi  wrote:
>> > Hi Rob,
>> >
>> > On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery 
>> > wrote:
>> >
>> >> Hi Guys,
>> >>
>> >> I've been testing out PGAgent 4 (build today from master at commit
>> >> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
>> >> handling on error is still fixed with the boost reimplementation
>> >> (which it is!)
>> >> and noticed a few issues compared to PGAgent 3 around feedback to the
>> >> user if the connection string is incorrect.
>> >>
>> >> I've tested with incorrect username and password, user not in the hba,
>> >> db doesn't exist, postgresql not running on the host (or incorrect
>> >> hostaddr) and 4 seems to always returns the error :
>> >> `
>> >> ERROR: Couldn't find the function 'pgagent_schema_version' - please
>> >> run pgagent_upgrade.sql.
>> >> `
>> >>
>> >> whereas 3.4.1 tends to output from the pgconnection itself like:
>> >> `
>> >> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
>> >> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
>> >> "172.30.0.16", user "pgagent_login_role", database
>> >> "pgagent_login_role", SSL on
>> >> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
>> >> "pgagent_login_role", database "pgagent_login_role", SSL off
>> >> `
>> >>
>> >> which at least points the user towards the actual error!
>> >>
>> >> Not sure if this is known already, but I thought I'd raise it prior to
>> >> release.
>> >>
>> > Thanks for sharing the information.
>> >
>> > Is it possible for you to share the logs with log level set to debug?
>> > Also - please share the operating system details?
>> >
>> > --
>> >
>> > Thanks & Regards,
>> >
>> > Ashesh Vashi
>> > EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> > 
>> >
>> >
>> > *http://www.linkedin.com/in/asheshvashi*
>> > 
>> >
>> >
>> >
>> >>
>> >> Thanks,
>> >> Rob
>> >> --
>> >>
>> >> --
>> >>  
>> >>
>> >>
>> >> Codeweavers May Newsletter
>> >>   l  Codeweavers
>> >> April
>> >> Finance Trends
>> >> 
>> >>
>> >>
>> >> April’s Dealer Highlights
>> >> 
>> >> > march-car-finance-trends
>> >> >
>> >>
>> >> _
>> >> _
>> >>
>> >>
>> >>
>> >> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> >> 
>> >> Codeweavers Ltd | Barn 4 | Dunston
>> >> Business Village | Dunston | ST18 9AB
>> >> Registered in England and Wales No.
>> >> 04092394 | VAT registration no. 974 9705 63
>> >>
>> >>
>> >>
>> >>
>> >> 
>> >> 
>> >> 
>> >> 
>> >>
>> >>
>> >>
>> >
>>
>> --
>>  
>>
>>
>> Codeweavers May Newsletter
>>   l  Codeweavers
>> April
>> Finance Trends
>> 
>>
>>
>> April’s Dealer Highlights
>> 
>> > >
>>
>> _
>> _
>>
>>
>>
>> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> 
>> Codeweavers Ltd | Barn 4 | Dunston
>> Business Village | Dunston | ST18 9AB
>> Registered in England and Wales No.
>> 04092394 | VAT registration no. 974 9705 63
>>
>>
>>
>>
>> 
>> 
>> 
>> 
>>
>>
>


-- 
Robert Emery
Infrastructure 

Re: PGAgent 4.0 error feedback

[Ashesh Vashi]

Hi Rob,


On Mon, Jun 11, 2018 at 12:50 PM, Rob Emery 
wrote:

> Hi Ashesh,
>
> Log-level wise I was using the same command for both versions:
>
>  ./pgagent -f -l 2 -t 1 -r 1 'hostaddr=server user=username
> password=password dbname=postgres'
>
> and messing up the connection string in various ways (missing dbname,
> incorrect password, incorrect hostaddr)
>
> This was built and tested on  Debian GNU/Linux 9.4 (stretch).
>
If you're working on the source, would please test the patch sent by me?

>
> I've attached an example of the differing output when the dbname is missing
> from the connectionstring; basically it's the same for all errors.
>

Thanks a lot for the information.

-- Thanks, Ashesh

>
> Many Thanks,
> Rob
>
> On 11/06/2018, Ashesh Vashi  wrote:
> > Hi Rob,
> >
> > On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery 
> > wrote:
> >
> >> Hi Guys,
> >>
> >> I've been testing out PGAgent 4 (build today from master at commit
> >> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
> >> handling on error is still fixed with the boost reimplementation
> >> (which it is!)
> >> and noticed a few issues compared to PGAgent 3 around feedback to the
> >> user if the connection string is incorrect.
> >>
> >> I've tested with incorrect username and password, user not in the hba,
> >> db doesn't exist, postgresql not running on the host (or incorrect
> >> hostaddr) and 4 seems to always returns the error :
> >> `
> >> ERROR: Couldn't find the function 'pgagent_schema_version' - please
> >> run pgagent_upgrade.sql.
> >> `
> >>
> >> whereas 3.4.1 tends to output from the pgconnection itself like:
> >> `
> >> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
> >> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
> >> "172.30.0.16", user "pgagent_login_role", database
> >> "pgagent_login_role", SSL on
> >> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
> >> "pgagent_login_role", database "pgagent_login_role", SSL off
> >> `
> >>
> >> which at least points the user towards the actual error!
> >>
> >> Not sure if this is known already, but I thought I'd raise it prior to
> >> release.
> >>
> > Thanks for sharing the information.
> >
> > Is it possible for you to share the logs with log level set to debug?
> > Also - please share the operating system details?
> >
> > --
> >
> > Thanks & Regards,
> >
> > Ashesh Vashi
> > EnterpriseDB INDIA: Enterprise PostgreSQL Company
> > 
> >
> >
> > *http://www.linkedin.com/in/asheshvashi*
> > 
> >
> >
> >
> >>
> >> Thanks,
> >> Rob
> >> --
> >>
> >> --
> >>  
> >>
> >>
> >> Codeweavers May Newsletter
> >>   l  Codeweavers
> >> April
> >> Finance Trends
> >> 
> >>
> >>
> >> April’s Dealer Highlights
> >> 
> >>  march-car-finance-trends
> >> >
> >>
> >> _
> >> _
> >>
> >>
> >>
> >> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
> >> 
> >> Codeweavers Ltd | Barn 4 | Dunston
> >> Business Village | Dunston | ST18 9AB
> >> Registered in England and Wales No.
> >> 04092394 | VAT registration no. 974 9705 63
> >>
> >>
> >>
> >>
> >> 
> >> 
> >> 
> >> 
> >>
> >>
> >>
> >
>
> --
>  
>
>
> Codeweavers May Newsletter
>   l  Codeweavers
> April
> Finance Trends
> 
>
>
> April’s Dealer Highlights
> 
>  >
>
> _
> _
>
>
>
> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
> 
> Codeweavers Ltd | Barn 4 | Dunston
> Business Village | Dunston | ST18 9AB
> Registered in England and Wales No.
> 04092394 | VAT registration no. 974 9705 63
>
>
>
>
> 
> 
> 
> 
>
>

Re: PGAgent 4.0 error feedback

[Rob Emery]

Hi Ashesh,

Log-level wise I was using the same command for both versions:

 ./pgagent -f -l 2 -t 1 -r 1 'hostaddr=server user=username
password=password dbname=postgres'

and messing up the connection string in various ways (missing dbname,
incorrect password, incorrect hostaddr)

This was built and tested on  Debian GNU/Linux 9.4 (stretch).

I've attached an example of the differing output when the dbname is missing
from the connectionstring; basically it's the same for all errors.

Many Thanks,
Rob

On 11/06/2018, Ashesh Vashi  wrote:
> Hi Rob,
>
> On Sun, Jun 10, 2018 at 2:26 AM, Rob Emery 
> wrote:
>
>> Hi Guys,
>>
>> I've been testing out PGAgent 4 (build today from master at commit
>> 86ca5c5ed1ad572075ba27e05e4680ebdf5b9feb) to check the connection
>> handling on error is still fixed with the boost reimplementation
>> (which it is!)
>> and noticed a few issues compared to PGAgent 3 around feedback to the
>> user if the connection string is incorrect.
>>
>> I've tested with incorrect username and password, user not in the hba,
>> db doesn't exist, postgresql not running on the host (or incorrect
>> hostaddr) and 4 seems to always returns the error :
>> `
>> ERROR: Couldn't find the function 'pgagent_schema_version' - please
>> run pgagent_upgrade.sql.
>> `
>>
>> whereas 3.4.1 tends to output from the pgconnection itself like:
>> `
>> Sat Jun  9 21:32:13 2018 : WARNING: Couldn't create the primary
>> connection (attempt 1): FATAL:  no pg_hba.conf entry for host
>> "172.30.0.16", user "pgagent_login_role", database
>> "pgagent_login_role", SSL on
>> FATAL:  no pg_hba.conf entry for host "172.30.0.16", user
>> "pgagent_login_role", database "pgagent_login_role", SSL off
>> `
>>
>> which at least points the user towards the actual error!
>>
>> Not sure if this is known already, but I thought I'd raise it prior to
>> release.
>>
> Thanks for sharing the information.
>
> Is it possible for you to share the logs with log level set to debug?
> Also - please share the operating system details?
>
> --
>
> Thanks & Regards,
>
> Ashesh Vashi
> EnterpriseDB INDIA: Enterprise PostgreSQL Company
> 
>
>
> *http://www.linkedin.com/in/asheshvashi*
> 
>
>
>
>>
>> Thanks,
>> Rob
>> --
>>
>> --
>>  
>>
>>
>> Codeweavers May Newsletter
>>   l  Codeweavers
>> April
>> Finance Trends
>> 
>>
>>
>> April’s Dealer Highlights
>> 
>> > >
>>
>> _
>> _
>>
>>
>>
>> *Phone:* 0800 021 0888   Email: contac...@codeweavers.net
>> 
>> Codeweavers Ltd | Barn 4 | Dunston
>> Business Village | Dunston | ST18 9AB
>> Registered in England and Wales No.
>> 04092394 | VAT registration no. 974 9705 63
>>
>>
>>
>>
>> 
>> 
>> 
>> 
>>
>>
>>
>

-- 
 


Codeweavers May Newsletter 
  l  Codeweavers April 
Finance Trends 



April’s Dealer Highlights 
 


_
_



*Phone:* 0800 021 0888   Email: contac...@codeweavers.net 

Codeweavers Ltd | Barn 4 | Dunston 
Business Village | Dunston | ST18 9AB
Registered in England and Wales No. 
04092394 | VAT registration no. 974 9705 63 



 
   
   
   


rob@ISIS:~/pgagent-git-build$ ./pgagent -f 'hostaddr=172.30.0.16 
user=pgagent_login_role password=password'
Mon Jun 11 08:17:49 2018 ERROR: Couldn't find the function 
'pgagent_schema_version' - please run pgagent_upgrade.sql.

rob@ISIS:~/pgagent-3.4.1-build$ ./pgagent -f 'hostaddr=172.30.0.16 
user=pgagent_login_role password=password'
Mon Jun 11 08:18:01 2018 : WARNING: Couldn't create the primary connection 
(attempt 1): could not connect to server: Connection refused
Is the server running on host "172.30.0.16" and accepting
TCP/IP connections on port 5432?