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

On Fri, Feb 19, 2016 at 6:45 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> 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.
>

OK. Will do that

>
>

>
>>
>> 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
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*