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

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-22 13:08:20
Message-ID: CA+OCxoxnNn6biSRVjMMSHvN4CPjVZPk8Za+UUhYX+BYZVfqKuw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks - patch applied.

On Mon, Feb 22, 2016 at 7:57 AM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi All
>
> I have modified the logic as per suggested by Dave. In new implementation
> I have added two methods "get_dependencies" and "get_dependents" to the
> class "PGChildNodeView". As per Ashesh's last commit all the
> view(respective node's view) classes is derived from "PGChildNodeView"
> instead of "NodeView", the above two methods will be directly accessible
> in the respective node's view class. If there is a change in the
> functionality to fetch dependencies/dependents for any node, developer of
> that node can easily overload the function and provide their own logic.
> Following are the changes need to be done to add this feature
>
> *Add following lines of code in** <your module>.js*
>
> - Add *hasDepends: true *in your file just like we have added *hasSQL*
> .
>
> *Add following lines of code in <your module>.__init__.py*
>
> Derive your view class from "PGChildNodeView" instead of "NodeView".
>
> Implement the dependents and dependencies function like below. Pass the
> appropriate node id to the functions "get_dependents" and
> "get_dependencies".
>
> @check_precondition
> def dependents(self, gid, sid, did, lid):
> """
> This function get the dependents and return ajax response
> for the language node.
>
> Args:
> gid: Server Group ID
> sid: Server ID
> did: Database ID
> lid: Language ID
> """
> dependents_result = self.get_dependents(self.conn, lid)
> return ajax_response(
> response=dependents_result,
> status=200
> )
>
> @check_precondition
> def dependencies(self, gid, sid, did, lid):
> """
> This function get the dependencies and return ajax response
> for the language node.
>
> Args:
> gid: Server Group ID
> sid: Server ID
> did: Database ID
> lid: Language ID
> """
> dependencies_result = self.get_dependencies(self.conn, lid)
> return ajax_response(
> response=dependencies_result,
> status=200
> )
>
> On Fri, Feb 19, 2016 at 6:51 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> 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-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>
>
>
>
> --
> *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

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2016-02-22 13:23:38 Re: [pgAdmin4] [Patch]: Language Module
Previous Message Dave Page 2016-02-22 13:07:34 pgAdmin 4 commit: Add support for displaying dependency and dependents