Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d7gmA-0003ka-CD for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 May 2017 11:24:26 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1d7gm9-00028H-Lk for pgadmin-hackers@arkaria.postgresql.org; Mon, 08 May 2017 11:24:25 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1d7gm8-00025L-Ee for pgadmin-hackers@postgresql.org; Mon, 08 May 2017 11:24:24 +0000 Received: from mail-pf0-x22d.google.com ([2607:f8b0:400e:c00::22d]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1d7gm3-0001M3-Je for pgadmin-hackers@postgresql.org; Mon, 08 May 2017 11:24:23 +0000 Received: by mail-pf0-x22d.google.com with SMTP id e64so31861730pfd.1 for ; Mon, 08 May 2017 04:24:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=qB4HvVzAK26mviC41sIrxbTJ66BJwtmiZ0DRG6nMl1g=; b=LIqq1FIa+GV4Y+0Nw+pVvSXzltvso94SccnIwCisHZpiitk6LCG9bcdTvJSYXf3Vws T2nPphainvHk8Qk9KgH/qh3TZC8iicIf/8hadxCYQCxVsEODYkcyNo3Tlpv9CNOMIG0a TWmCv1c4ickdhVfgjw+Pe6HZX7J4yGhMsXw+3WwAGJAYmuBqCNV8PXPa+NJu5kaduwpw u2z4xVJVH1qauY34msu9bi0Tf0OJAVkNCrDyms6vrRflrcNY/JGT8HaIUiAM0mGmCcSa fLtMnlrpbdu6252WD9nFYgyLd3WVnGcxJa+DdVbIKl/MvRWmA3IXHcR/4VKdQuqEDLmh 8ZZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=qB4HvVzAK26mviC41sIrxbTJ66BJwtmiZ0DRG6nMl1g=; b=TdfliLKDyNa9vMypUTJdj1uKmdB7nFs3LBKFEBPoDnSQ1xyBRExTcalLtB48YWeLY7 M7Clkdjl0o4Zjvol3wOk/u850aTS2yluN/E3jHFQF47kM86vYlbkWo6PyTLsdrfmcD2B qx1znSLVqX7+62XDdAU+QvcuWyD6gTJeISDBvh+RK3pDg1Xc1t+9X3Cy5+R5/LY9HsBD gonqTMQWcQlmbZ06oYAaKl/j3fUO5ifh8CPjRt5bWFwSqmSWOr2PL09m6xxZrRk6yezg ZfpzWXlw2bcaxDswl6v4wrv2CVuV3SMMMAgR7mPF7HRylje626qqyVbX/NzBWRFYe52Z 5PSg== X-Gm-Message-State: AN3rC/6/tFMG/VrseOnUtQLGF71NK2Cjqa0hfakk+CGkHADGfUH9UTVH zQC4MDtFoxsVxNL9LE39xvXiJP2tZGbQ X-Received: by 10.99.116.2 with SMTP id p2mr18146019pgc.74.1494242657325; Mon, 08 May 2017 04:24:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.170.75 with HTTP; Mon, 8 May 2017 04:24:16 -0700 (PDT) In-Reply-To: References: From: Akshay Joshi Date: Mon, 8 May 2017 16:54:16 +0530 Message-ID: Subject: Re: [patch] Dependents and Dependencies in GreenPlum To: Dave Page Cc: Robert Eckhardt , Sarah McAlear , Joao Pedro De Almeida Pereira , Ashesh Vashi , pgadmin-hackers , Matthew Kleiman Content-Type: multipart/alternative; boundary=f403045c63a2cc6e3d054f017aac X-Pg-Spam-Score: -1.9 (-) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --f403045c63a2cc6e3d054f017aac Content-Type: text/plain; charset=UTF-8 Hi All I have reviewed the code and have following questions: - Why do we have empty __init__.py file in sql and templates/ folder. In this patch we have couple of occurrences one of them is web/ pgadmin/browser/server_groups/servers/databases/schemas/table/templates/trigger and web/pgadmin/browser/server_groups/servers/databases/schemas /table/templates/trigger/sql - Why do we have tests folder inside sql folder as we already have tests folder in respective module. For example we have tests folder in *web/pgadmin/browser/server_groups/servers/databases/schemas/table/column *then why do we need it in *web/pgadmin/browser/server_groups/servers/databases/schemas/table/templates/column/sql/tests* Apart from above code looks good to me. On Mon, May 8, 2017 at 2:20 PM, Akshay Joshi wrote: > > > On Mon, May 8, 2017 at 1:46 PM, Dave Page wrote: > >> Akshay, as Ashesh is unavailable today, can you please review/commit this >> ASAP? >> > > Sure. > >> >> Thanks. >> >> On Fri, May 5, 2017 at 1:18 PM, Dave Page wrote: >> >>> Can you get this polished off on Monday please Ashesh? >>> >>> On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt >>> wrote: >>> >>>> All, >>>> >>>> This change in the xss testing is preventing our CI from going green >>>> and is also preventing the Dependents and Dependencies tabs in Greenplum >>>> from being useful. >>>> >>>> Can we either merge this or provide feedback as to what needs to change >>>> so that it can be merged. >>>> >>>> Thank you >>>> Rob >>>> >>>> On Tue, May 2, 2017 at 5:17 PM, Sarah McAlear >>>> wrote: >>>> >>>>> Hi Hackers & Ashesh! >>>>> >>>>> Is there anything else we can do for this? >>>>> >>>>> Thanks! >>>>> Matt & Sarah >>>>> >>>>> On Thu, Apr 27, 2017 at 10:37 AM, Joao Pedro De Almeida Pereira < >>>>> jdealmeidapereira@pivotal.io> wrote: >>>>> >>>>>> Thanks for reviewing, Ashesh. >>>>>> >>>>>> We have updated the patch. The headers are all consistent and we >>>>>> removed the __init__.py files in directories containing only .sql. >>>>>> >>>>>> Thanks! >>>>>> Joao & Matt >>>>>> >>>>>> On Wed, Apr 26, 2017 at 11:22 AM, Joao Pedro De Almeida Pereira < >>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>> >>>>>>> Hello Ashesh, >>>>>>> >>>>>>> Thanks for reviewing the patch. >>>>>>> >>>>>>> We added the __init__.py files into templates to convert them into >>>>>>> packages so that the tests inside of them can be found by the test runner. >>>>>>> >>>>>>> Thanks! >>>>>>> Joao & Sarah >>>>>>> >>>>>>> On Wed, Apr 26, 2017 at 1:26 AM, Ashesh Vashi < >>>>>>> ashesh.vashi@enterprisedb.com> wrote: >>>>>>> >>>>>>>> On Mon, Apr 24, 2017 at 4:43 PM, Dave Page >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Ashesh, can you review/commit this please? >>>>>>>>> >>>>>>>>> On Fri, Apr 21, 2017 at 8:42 PM, Joao Pedro De Almeida Pereira < >>>>>>>>> jdealmeidapereira@pivotal.io> wrote: >>>>>>>>> >>>>>>>>>> Hi Hackers, >>>>>>>>>> >>>>>>>>>> We found out that when you are connected to a GreenPlum database >>>>>>>>>> and try to get Dependents and Dependencies of an object the application was >>>>>>>>>> returning a SQL error. >>>>>>>>>> >>>>>>>>>> This patch splits the SQL query used to retrieve the Dependents, >>>>>>>>>> Dependencies, and Roles SQL file into multiple versioned files. >>>>>>>>>> Add Unit Tests for each file. >>>>>>>>>> Also added __init__.py files to other test directories to run the >>>>>>>>>> tests in them. >>>>>>>>>> >>>>>>>>> Hi Joao & Sarah, >>>>>>>> >>>>>>>> Why do we need to add __init__.py in the template directory? >>>>>>>> I didn't understand the purpose of the adding __init__.py files in >>>>>>>> the template directories. >>>>>>>> >>>>>>>> NOTE: The headers in those files are not consistent with the other >>>>>>>> project files. >>>>>>>> >>>>>>>> -- >>>>>>>> >>>>>>>> Thanks & Regards, >>>>>>>> >>>>>>>> Ashesh Vashi >>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> *http://www.linkedin.com/in/asheshvashi* >>>>>>>> >>>>>>>> >>>>>>>>> Add ORDER BY into Copy Selection Feature test to ensure the >>>>>>>>>> results are retrieved always in the same order >>>>>>>>>> Renamed the Scenario of the xss_checks_pgadmin_debugger_test and >>>>>>>>>> skip it for versions less than 9.1 >>>>>>>>>> >>>>>>>>>> Thanks >>>>>>>>>> >>>>>>>>>> Joao & Sarah >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Sent via pgadmin-hackers mailing list ( >>>>>>>>>> pgadmin-hackers@postgresql.org) >>>>>>>>>> To make changes to your subscription: >>>>>>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Dave Page >>>>>>>>> Blog: http://pgsnake.blogspot.com >>>>>>>>> Twitter: @pgsnake >>>>>>>>> >>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com >>>>>>>>> The Enterprise PostgreSQL Company >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org >>>>>> ) >>>>>> To make changes to your subscription: >>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers >>>>>> >>>>>> >>>>> >>>> >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> >> >> -- >> 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* > -- *Akshay Joshi* *Principal Software Engineer * *Phone: +91 20-3058-9517Mobile: +91 976-788-8246* --f403045c63a2cc6e3d054f017aac Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi All

I have reviewed the code and hav= e following questions:
  • Why do we have empty __init__.py file in sql and templates/= <module> folder. In this patch we have couple of occurrences one of t= hem is web/pgadm= in/browser/server_groups/servers/databases/schemas/table/templates/trigger an= d=C2=A0=C2=A0web/pgadmin/browser/server_groups/servers/databases/schemas/table/templa= tes/trigger/sql<= /span>
  • Why do we have tests folder inside sql folder as we already have test= s folder in respective module. For example we have tests folder in=C2=A0= web/pgadmin/browser/server_groups/servers/databases/schemas/table/column=C2=A0then why do= we need it in=C2=A0web/pgadmin/browser/server_groups/servers/databases/schemas/table/temp= lates/column/sql= /tests
Apart from above code looks good to me.

On Mon, May 8,= 2017 at 2:20 PM, Akshay Joshi <akshay.joshi@enterprisedb.com<= /a>> wrote:

On Mon, May 8, 2017 at 1:46 PM, Dave Page <dpage@pgadmin.org> wrote:
Akshay, as = Ashesh is unavailable today, can you please review/commit this ASAP?
<= /blockquote>

=C2=A0 =C2=A0 Sure.=C2=A0
=

=
Thanks.
<= div class=3D"m_-2092361783909953136h5">

On Fri, May 5, 2017 at 1:18 PM, Dave Page <dpage@p= gadmin.org> wrote:
Can you get this polished off on Monday please Ashesh?

<= div class=3D"gmail_quote">On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt = <reckhardt@pivotal.io> wrote:
All,=C2=A0

This change in the xs= s testing is preventing our CI from going green and is also preventing the= =C2=A0Dependents and Dependencies tabs in Greenplum from being useful.=C2= =A0

Can we either merge this or provide feedback a= s to what needs to change so that it can be merged.=C2=A0

Thank you
Rob

On Tue, May 2, 2017 at 5:17 PM,= Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Hackers & Ashesh!

I= s there anything else we can do for this?

Thanks!<= br>Matt & Sarah

On Thu, Apr 27, 2017 at 10:37 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Thanks for reviewing, Ashesh.

=
We have updated the patch. The headers are all consistent and we remov= ed the __init__.py files in dire= ctories containing only .sql.

Thanks!
Joao & Matt

On Wed, Apr 26, 2017 at 11:22 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Ashesh,

Thanks for revie= wing the patch.

We added the __init__.py files int= o templates to convert them into packages so that the tests inside of them = can be found by the test runner.

Thanks!
= Joao & Sarah

On Wed, Apr 26, 2017 at 1:26 AM, Ashesh Vashi <ashesh.vas= hi@enterprisedb.com> wrote:
On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org&= gt; wrote:
Ashesh, can you review/commit this please?

On Fri, Apr 21, 2017 at 8:42 PM, Joao Pedro De Almeida Pe= reira <jdealmeidapereira@pivotal.io> wrote:
Hi Hackers,

We found out that when you are connected to a GreenPlum database and= try to get Dependents and Dependencies of an object the application was re= turning a SQL error.

This patch splits the SQL que= ry used to retrieve the Dependents, Dependencies, and Roles SQL file into m= ultiple versioned files.
Add Unit Tests for each file.
= Also added __init__.py files to other test directories to run the tests in = them.
H= i Joao & Sarah,

Why do we need to add __init__.py in= the template directory?
I didn't understand the purpose of t= he adding __init__.py files in the template directories.

NOTE: The headers in those files are not consistent with the other p= roject files.

<= p style=3D"margin:0pt">--

= Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:=C2=A0Enterprise PostgreSQL Company


http://www.linkedin.com/in/ashe= shvashi

Add ORDER BY int= o Copy Selection Feature test to ensure the results are retrieved always in= the same order
Renamed the Scenario of the=C2=A0xss_checks_pgadm= in_debugger_test and skip it for versions less than 9.1

=
Thanks

Joao & Sarah


--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-ha= ckers




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

Ent= erpriseDB UK: htt= p://www.enterprisedb.com
The Enterprise PostgreSQL Company





--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-ha= ckers






--
=
Dave Page
Blog: <= a href=3D"http://pgsnake.blogspot.com" target=3D"_blank">http://pgsnake.blo= gspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.comThe Enterprise PostgreSQL Company



--
=
Dave Page
Blog: http://pgsnake.blogspot.com
Twitt= er: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postg= reSQL Company



--
Ak= shay Joshi
Principal Software Engineer=C2=A0
=

<= /font>

Phone: +91 20-3058-9517
Mob= ile: +91 976-788-8246



--
Akshay Joshi


<= b>Phone: +91 20-3058-9517
Mobile: +91 976-788= -8246
--f403045c63a2cc6e3d054f017aac--