On Fri, Feb 19, 2016 at 6:45 PM, Dave Page <[email protected]> wrote:
> > > On Fri, Feb 19, 2016 at 1:09 PM, Akshay Joshi < > [email protected]> wrote: > >> Hi Dave >> >> On Fri, Feb 19, 2016 at 5:45 PM, Dave Page <[email protected]> wrote: >> >>> Hi >>> >>> On Fri, Feb 19, 2016 at 10:17 AM, Akshay Joshi < >>> [email protected]> 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*
