Re: [pgAdmin4][Patch]: Dependecies/Dependents functionality

On Fri, Feb 19, 2016 at 1:09 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi Dave
>
> On Fri, Feb 19, 2016 at 5:45 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> 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.
>>
>
> If I understand you correctly, you want me to remove logic to fetch
> dependencies/dependent for the "Tablespace" and "Role" node from the web/
> pgadmin/browser/server_groups/servers/depends.py and added it into the
> respective nodes? Logic to fetch Dependents is different for "Tablespace"
> and "Role", but to fetch Dependencies logic is generic for all the nodes(
> for sequence node we have one extra query). Similarly to fetch dependents
> we will have one extra query for Table and Column Node, so you want me to
> remove that part as well? in that case it's responsibility of the developer
> who will implement the Table/Column/Sequence node to add that logic later?
>

Yes. Each node should be self-contained, and not include any non-generic
code relating to other nodes.

>
> I have followed the pgAdmin3 code here, and also thought that it's
> better to have whole logic to fetch Dependencies/Dependent in a single
> file, instead of having some code (generic logic) in one file(depends.py)
> and some part in respective modules __init__.py.
>

Well you can still have only a single copy of any generic code - put it in
a base class and have nodes inherit that.

We're trying to fix (and change, by making things modular) some of the
design choices in pgAdmin 3, so it's not necessarily a good idea to copy
code structure from there.

>
>> 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.
>>
>
> Sure, I'll do that.
>

Thanks.

>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>

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

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