Re: [pgadmin-hackers][patch] Dependents and Dependencies in GreenPlum - Mailing list pgadmin-hackers
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 & SarahOn Mon, May 8, 2017 at 7:28 AM, Dave Page <dpage@pgadmin.org> wrote:HiOn Mon, May 8, 2017 at 12:24 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote: Hi AllI 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_gro
ups/servers/databases/schemas/ table/templates/trigger and web/pgadmin/browser/serve r_groups/servers/databases/sch emas/table/templates/trigger/s ql 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/schem as/table/column then why do we need it in web/pgadmin/browser/server_ groups/servers/databases/schem as/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 youRobOn 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 & SarahOn 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 & MattOn 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 & SarahOn 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:Hi Joao & Sarah,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.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.--Add ORDER BY into Copy Selection Feature test to ensure the results are retrieved always in the same orderRenamed the Scenario of the xss_checks_pgadmin_debugger_test and skip it for versions less than 9.1 ThanksJoao & 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------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
Mobile: +91 976-788-8246
pgadmin-hackers by date: