[pgadmin-hackers] Jenkins build is back to normal : pgadmin4-master-python34 #106

[pgAdmin 4 Jenkins]

See 




-- 
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

[Akshay Joshi]

Thanks patch applied.

On Mon, May 15, 2017 at 6:19 AM, Matthew Kleiman 
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.jo...@enterprisedb.com> wrote:
>
>> Hi Joao
>>
>> On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <
>> jdealmeidapere...@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 <
>>> jdealmeidapere...@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.va...@enterprisedb.com> wrote:

> On Thu, May 11, 2017 at 12:03 PM, Dave Page  wrote:
>
>>
>>
>> On 11 May 2017, at 07:11, Akshay Joshi 
>> wrote:
>>
>> Hi Sarah
>>
>> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear 
>> 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.
>>
>>

[pgadmin-hackers] pgAdmin 4 commit: 1) Splits the SQL query used to retrieve the Dependen

[Akshay Joshi]

1) Splits the SQL query used to retrieve the Dependents, Dependencies, and 
Roles SQL file into multiple versioned files.
2) Add Unit Tests for each file.
3) Add ORDER BY into Copy Selection Feature test to ensure the results are 
retrieved always in the same order
4) Renamed the Scenario of the xss_checks_pgadmin_debugger_test and skip it for 
versions less than 9.1
5) Deleted unused __init__.py files.

Branch
--
master

Details
---
https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=3bf17d9df42f2fd2c9a4588487655e4078974104
Author: Joao Pedro De Almeida Pereira 

Modified Files
--
.../databases/schemas/tables/templates/__init__.py |  0
.../schemas/tables/templates/column/__init__.py|  0
.../templates/column/sql/9.2_plus/__init__.py  |  0
.../templates/column/sql/default/__init__.py   |  0
.../templates/column/sql/tests/test_column_acl.py  | 47 
.../column/sql/tests/test_column_properties.py | 56 --
.../schemas/tables/templates/table/__init__.py |  0
.../schemas/tables/templates/table/sql/__init__.py |  0
.../tables/templates/table/sql/tests/__init__.py   |  0
.../templates/table/sql/tests/test_tables_acl.py   | 66 -
.../templates/table/sql/tests/test_tables_node.py  | 74 ---
.../table/sql/tests/test_tables_properties.py  | 77 
.../templates/trigger/sql/9.1_plus/__init__.py |  0
.../tables/templates/trigger/sql/__init__.py   |  0
.../templates/trigger/sql/default/__init__.py  |  0
.../tables/templates/trigger/sql/tests/__init__.py |  0
.../trigger/sql/tests/test_trigger_get_oid.py  | 60 ---
.../trigger/sql/tests/test_trigger_nodes.py| 51 -
.../schemas/tables/tests/test_column_acl_sql.py| 51 +
.../tables/tests/test_column_properties_sql.py | 52 +
.../schemas/tables/tests/test_tables_acl_sql.py| 54 ++
.../schemas/tables/tests/test_tables_node_sql.py   | 55 ++
.../tables/tests/test_tables_properties_sql.py | 72 ++
.../tables/tests/test_trigger_get_oid_sql.py   | 52 +
.../schemas/tables/tests/test_trigger_nodes_sql.py | 43 +++
.../depends/sql/9.1_plus/dependencies.sql  | 45 
.../templates/depends/sql/9.1_plus/dependents.sql  | 44 +++
.../templates/depends/sql/default/dependencies.sql | 42 +++
.../templates/depends/sql/default/dependents.sql   | 66 +
.../depends/sql/default/role_dependencies.sql  |  4 +
.../servers/tests/test_dependencies_sql.py | 51 +
.../servers/tests/test_dependents_sql.py   | 51 +
.../servers/tests/test_role_dependencies_sql.py| 85 ++
web/pgadmin/browser/utils.py   | 10 +--
.../copy_selected_query_results_feature_test.py| 11 ++-
.../xss_checks_pgadmin_debugger_test.py|  6 +-
.../python_test_utils/sql_template_test_base.py| 59 +++
37 files changed, 783 insertions(+), 501 deletions(-)


-- 
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

[Akshay Joshi]

Hi Joao

On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira <
jdealmeidapere...@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 <
> jdealmeidapere...@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.va...@enterprisedb.com> wrote:
>>
>>> On Thu, May 11, 2017 at 12:03 PM, Dave Page  wrote:
>>>


 On 11 May 2017, at 07:11, Akshay Joshi 
 wrote:

 Hi Sarah

 On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear 
 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.jo...@enterprisedb.com> wrote:
>
>>
>>
>> On May 10, 2017 19:07, "Sarah McAlear"  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 <