Обсуждение: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

Поиск
Список
Период
Сортировка

[pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Joao Pedro De Almeida Pereira
Дата:
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.
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
Вложения

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Dave Page
Дата:
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.
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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Ashesh Vashi
Дата:
On Mon, Apr 24, 2017 at 4:43 PM, Dave Page <dpage@pgadmin.org> 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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Joao Pedro De Almeida Pereira
Дата:
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 <dpage@pgadmin.org> 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


Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Joao Pedro De Almeida Pereira
Дата:
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 <dpage@pgadmin.org> 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



Вложения

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Sarah McAlear
Дата:
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 <dpage@pgadmin.org> 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


Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Robert Eckhardt
Дата:
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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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



Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Dave Page
Дата:
Can you get this polished off on Monday please Ashesh?

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Dave Page
Дата:
Akshay, as Ashesh is unavailable today, can you please review/commit this ASAP?

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:


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?

    Sure. 

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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-9517
Mobile: +91 976-788-8246

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Hi All

I have reviewed the code and have 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 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 <akshay.joshi@enterprisedb.com> 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?

    Sure. 

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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-9517
Mobile: +91 976-788-8246



--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Dave Page
Дата:
Hi

On Mon, May 8, 2017 at 12:24 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All

I have reviewed the code and have 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 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
That was already answered up-thread: 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.
  • 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
That does seem like a valid concern to me.
 
Apart from above code looks good to me.

On Mon, May 8, 2017 at 2:20 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> 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?

    Sure. 

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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 





--
Akshay Joshi
Principal Software Engineer 





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

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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Sarah McAlear
Дата:
Hi!

This patch has the tests moved to the test directories of the parent. 

Thanks,
Joao & Sarah

On Mon, May 8, 2017 at 7:28 AM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, May 8, 2017 at 12:24 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All

I have reviewed the code and have 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 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
That was already answered up-thread: 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.
  • 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
That does seem like a valid concern to me.
 
Apart from above code looks good to me.

On Mon, May 8, 2017 at 2:20 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> 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?

    Sure. 

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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 





--
Akshay Joshi
Principal Software Engineer 





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

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

Вложения

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Hi Sarah

Below are my review comments on the latest patch:
  • Some test file names ended with "_sql_template.py" do we need to add that string ? 
  • Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
  • Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
  • Following test cases are failing for PG 9.6 and PG 10.0 with error "name 'long' is not defined". I am using python 3.5
    • TestRoleDependenciesSqlTemplate
    • TestTablesPropertiesSqlTemplate
    • TestTablesNodeSqlTemplate

On Mon, May 8, 2017 at 9:44 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi!

This patch has the tests moved to the test directories of the parent. 

Thanks,
Joao & Sarah

On Mon, May 8, 2017 at 7:28 AM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Mon, May 8, 2017 at 12:24 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All

I have reviewed the code and have 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 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
That was already answered up-thread: 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.
  • 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
That does seem like a valid concern to me.
 
Apart from above code looks good to me.

On Mon, May 8, 2017 at 2:20 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> 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?

    Sure. 

Thanks.

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

On Thu, May 4, 2017 at 12:51 PM, Robert Eckhardt <reckhardt@pivotal.io> 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 <smcalear@pivotal.io> 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 <dpage@pgadmin.org> 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 





--
Akshay Joshi
Principal Software Engineer 





--
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
Mobile: +91 976-788-8246

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Sarah McAlear
Дата:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?

Thanks,
George & Sarah

Вложения

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Sarah McAlear
Дата:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 



Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 




Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Sarah McAlear
Дата:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 





Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Dave Page
Дата:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Khushboo Vashi
Дата:


On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Joao Pedro De Almeida Pereira
Дата:

Hello all,
This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently of the user that is used on the test.

Thanks
Joao

On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Joao Pedro De Almeida Pereira
Дата:
Hello Akshay,

You can find attached a new version of the patch that changes the test in question. 
This new version instead of requiring a creation of a new Role now does the following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:

Hello all,
This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently of the user that is used on the test.

Thanks
Joao


On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 



Вложения

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Hi Joao

On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Akshay,

You can find attached a new version of the patch that changes the test in question. 
This new version instead of requiring a creation of a new Role now does the following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

   Test case still failing on my machine because when we create new Role it is without password and when we try to create table with that role first we will try to connect the database using that new role with servers['db_password'] which is wrong. I have modified setup function of " test_role_dependencies_sql.py" and it works, we can create new role using database server's password 

Old setup function:
      def setUp(self):        

    create_role(self.server, "testpgadmin")
    self.server_with_modified_user = self.server.copy()
    self.server_with_modified_user['username'] = "testpgadmin"


Modified setup function:


   def setUp(self):

    with test_utils.Database(self.server) as (connection, database_name):
cursor = connection.cursor()
try:
cursor.execute(
"CREATE ROLE testpgadmin LOGIN PASSWORD '%s'"
% self.server['db_password'])
except Exception as exception:
print(exception)
connection.commit()

self.server_with_modified_user = self.server.copy()
self.server_with_modified_user['username'] = "testpgadmin"

   If the above logic looks good to you then I'll commit the code.

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:

Hello all,
This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently of the user that is used on the test.

Thanks
Joao


On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 






--
Akshay Joshi
Principal Software Engineer 


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

Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Matthew Kleiman
Дата:
Hi Akshay

I really appreciate you taking a look at this on a Sunday! 
I am not able to get access to my work computer to run your changes directly. From what you are saying, though, it sounds like a reasonable change. If this change is passing in your environment, I say go ahead and make the change to the patch and commit.

Thanks again for helping us get this into the release. 

Best,
Matt

On Sun, May 14, 2017 at 4:52 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao

On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Akshay,

You can find attached a new version of the patch that changes the test in question. 
This new version instead of requiring a creation of a new Role now does the following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

   Test case still failing on my machine because when we create new Role it is without password and when we try to create table with that role first we will try to connect the database using that new role with servers['db_password'] which is wrong. I have modified setup function of " test_role_dependencies_sql.py" and it works, we can create new role using database server's password 

Old setup function:
      def setUp(self):        

    create_role(self.server, "testpgadmin")
    self.server_with_modified_user = self.server.copy()
    self.server_with_modified_user['username'] = "testpgadmin"


Modified setup function:


   def setUp(self):

    with test_utils.Database(self.server) as (connection, database_name):
cursor = connection.cursor()
try:
cursor.execute(
"CREATE ROLE testpgadmin LOGIN PASSWORD '%s'"
% self.server['db_password'])
except Exception as exception:
print(exception)
connection.commit()

self.server_with_modified_user = self.server.copy()
self.server_with_modified_user['username'] = "testpgadmin"

   If the above logic looks good to you then I'll commit the code.

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:

Hello all,
This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently of the user that is used on the test.

Thanks
Joao


On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 






--
Akshay Joshi
Principal Software Engineer 



Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum

От
Akshay Joshi
Дата:
Thanks patch applied.

On Mon, May 15, 2017 at 6:19 AM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
Hi Akshay

I really appreciate you taking a look at this on a Sunday! 
I am not able to get access to my work computer to run your changes directly. From what you are saying, though, it sounds like a reasonable change. If this change is passing in your environment, I say go ahead and make the change to the patch and commit.

Thanks again for helping us get this into the release. 

Best,
Matt

On Sun, May 14, 2017 at 4:52 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Joao

On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Akshay,

You can find attached a new version of the patch that changes the test in question. 
This new version instead of requiring a creation of a new Role now does the following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

   Test case still failing on my machine because when we create new Role it is without password and when we try to create table with that role first we will try to connect the database using that new role with servers['db_password'] which is wrong. I have modified setup function of " test_role_dependencies_sql.py" and it works, we can create new role using database server's password 

Old setup function:
      def setUp(self):        

    create_role(self.server, "testpgadmin")
    self.server_with_modified_user = self.server.copy()
    self.server_with_modified_user['username'] = "testpgadmin"


Modified setup function:


   def setUp(self):

    with test_utils.Database(self.server) as (connection, database_name):
cursor = connection.cursor()
try:
cursor.execute(
"CREATE ROLE testpgadmin LOGIN PASSWORD '%s'"
% self.server['db_password'])
except Exception as exception:
print(exception)
connection.commit()

self.server_with_modified_user = self.server.copy()
self.server_with_modified_user['username'] = "testpgadmin"

   If the above logic looks good to you then I'll commit the code.

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:

Hello all,
This test is trying to check if the sql to retrieve the role dependencies of a object in the database is correct. Using a different user to connect to the database and create tables there is no need for extra setup because the role depedencies table entry is added automatically. Also we talked with some internal users and they told us they always used a different user to connect to the database with pgadmin.

After your comments we realize that the test need to have a different setup in order to become more readable and resilient. What do we need to do in the test setup to ensure an entry is added to the role dependency table?

Also the test need to be a little bit more resilient to work independently of the user that is used on the test.

Thanks
Joao


On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote:


On 11 May 2017, at 07:11, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Sarah

On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Ashkay!

TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0

We experienced a similar problem when we started developing this patch. We noticed that we were connecting to the database with the super user (the db owner user), which did not create the role we are expecting when a table is created. To ensure that this role is created, we had to create a new user and use it to execute all the tests. After we made this change, the tests passed. 

Can you try to create a new user and use it to execute your tests?

    I did that and it works. I have created new user 'test' with superuser privileges and update parameter db_username="test" in test_config.json file. But still I am unable to understand why it doesn't work with 'postgres' (default) user?  

Agreed. That suggests something fishy is going on that should be understood.


We don't check role dependencies for every object. As per the code, the role dependencies are executed for columns not for the table itself.
And in the test cases, the table object is trying to execute the role dependencies, so I think this test case is not correct.

Thanks,
João & Sarah

On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On May 10, 2017 19:07, "Sarah McAlear" <smcalear@pivotal.io> wrote:
Hi Akshay!

Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our tests pass.

    On both 9.6 and 10.0. Query returned 0 rows which is compared against 1 in assert statement. 

Thanks!
João & Sarah

On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Sarah

On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear@pivotal.io> wrote:
Hi Akshay!
 
Some test file names ended with "_sql_template.py" do we need to add that string ? 
we added this suffix after moving the tests up a level to tables/tests to clarify what subject they applied to. we changed the suffix to "_sql.py"

- Files "test_column_acl_sql_template.py" and "test_column_properties_sql_template.py" should be moved from tables->tests to tables->column->tests. As it's related to column.
- Files "test_trigger_get_oid_sql_template.py" and "test_trigger_nodes_sql_template.py" should be moved from tables->tests to tables->triggers->tests. As its related to triggers.
these tests are related to the sql files in tables/templates/column, not tables/column, so moving them into tables/column would be more confusing. 

Following test cases are failing
Thank you, fixed, see new patch. Can you confirm that TestTablesNodeSql doesn't fail in your environment?
   
    TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with below error:
    .....\test_role_dependencies_sql.py\", line 41, in assertions\n self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0


Thanks,
George & Sarah




--
Akshay Joshi
Principal Software Engineer 








--
Akshay Joshi
Principal Software Engineer 






--
Akshay Joshi
Principal Software Engineer 






--
Akshay Joshi
Principal Software Engineer 


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