From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4][Patch]: Dependecies/Dependents functionality |
Date: | 2016-02-19 12:15:42 |
Message-ID: | CA+OCxoxOwarr3LjZQL0q6OJ6E+=WEN2GPXK-OVwVE8+TrLtFDg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi
On Fri, Feb 19, 2016 at 10:17 AM, Akshay Joshi <
akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
> Hi All
>
> I have implemented the logic to show Dependencies/Dependents when user
> click on browser nodes. Attached is the patch file, please review it and if
> it looks good then please commit it.
>
> I have also added this support in "Database" , "Tablespace" and "Role" as
> this nodes are already been committed. Following is the information about
> how to add the support in other nodes
>
It seems to me that this patch is implemented in a way to violates
modularity - specifically, it adds smarts about a child node into the code
for a parent node, rather than leaving each node to be self-contained. For
example, web/pgadmin/browser/server_groups/servers/depends.py contains the
SQL and other logic for displaying the dependencies and dependents of
tablespace and role nodes.
I would expect to see role-specific code confined to the roles module,
database-specific confined to the database module and so on.
As a side-note - shouldn't the code also use SQL templates for the SQL,
rather than hard-coding it into Python? I would expect to see it using the
same versioning mechanism we use elsewhere, to make it a simple matter of
adding a new template to update to support a new version of Postgres.
Thanks.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2016-02-19 13:09:07 | Re: [pgAdmin4][Patch]: Dependecies/Dependents functionality |
Previous Message | Sanket Mehta | 2016-02-19 11:03:50 | Re: patch for cast module |