You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rhtyd <gi...@git.apache.org> on 2016/05/18 08:17:55 UTC
[GitHub] cloudstack pull request: dynamic-roles: remove unnecessary order b...
GitHub user rhtyd opened a pull request:
https://github.com/apache/cloudstack/pull/1551
dynamic-roles: remove unnecessary order by ID
On some MySQL server envs, this may cause a SQL statement error, though
I was unable to reproduce it. Since it's not needed, an order by 'sort_order'
is enough, we can safely remove it.
/cc @swill @anshulgangwar @DaanHoogland and others
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shapeblue/cloudstack rbac-mysql-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1551.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1551
----
commit 1770f3cb86f84128c40571330af55a7255fa49cc
Author: Rohit Yadav <ro...@shapeblue.com>
Date: 2016-05-18T08:16:18Z
dynamic-roles: remove unnecessary order by ID
On some MySQL server envs, this may cause a SQL statement error, though
I was unable to reproduce it. Since it's not needed, an order by 'sort_order'
is enough, we can safely remove it.
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220675652
Thanks @swill if we keep getting this error, I'll try to find some time to see what's causing this as well
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: remove unnecessary order b...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220091668
@swill I got a report that the GenericDaoBase generate query with multiple filters (order by clauses) may fail for MySQL:
SELECT role_permissions.id, role_permissions.uuid, role_permissions.role_id, role_permissions.rule, role_permissions.permission, role_permissions.description, role_permissions.sort_order FROM role_permissions ORDER BY role_permissions.sort_order ASC role_permissions.id ASC;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'role_permissions.id ASC' at line 1
This was reported for mysql-community-server-5.6.23-3.el7.x86_64, though I was unable to reproduce the issue with either MySQL 5.6 or 5.7 (Travis uses 5.5, and on 5.5 too we don't see any issues).
Nevertheless, the `ORDER BY id` is not needed for the functionality; in the method we need ORDER BY sort_order i.e return list of role permission by the order set by the root admin.
Will, I don't see this as a problem but as this is not necessary therefore can be safely removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220512003
These errors are not related to this PR, but if you know where these errors are coming from I would be interested in some input. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220626559
@rhtyd thank you. I will review what happened there. This is something that has started showing up intermittently, but only started happening in the last few weeks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: remove unnecessary order b...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220236481
@swill I've tested dynamic-roles features both with maven and rpms. I've tested the rpm/deb packages, including upgrade cases using this repository: (latest master and this PR) https://packages.shapeblue.com/cloudstack/custom/master-latest/
I've included some packaging related fixes in this PR that fix following (that could be done manually, but making it automated provides a better upgrade experience for the user):
- For rpms, copies commands.properties to /etc/cloudstack/management in case of an existing installation. In case of debian packages, the commands.properties file is already installed at the /etc path. This ensures people will continue to use static-checker after the upgrade, and commands.properties file will exist at /etc/cloudstack/management.
- Reduces the cloudstack-common rpm size by half, by no bundling systemvm.zip file. This file is not needed and we don't package it in debian packages.
- Since #1054 we're using mysql connector python, the last patch fixes the dynamic-roles migration tool/script to use mysql.connector as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220395388
I already started another run against this PR, so that should also validate nothing was accidentally broken.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220273077
\U0001f44d (code review only)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: remove unnecessary order b...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220030012
Do you have reports already of this causing problems?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220716022
I think since it is an intermittent issue, we did not catch it on the actual PR it was introduced in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220511935
### CI RESULTS
```
Tests Run: 82
Skipped: 0
Failed: 0
Errors: 3
Duration: 8h 29m 09s
```
**Summary of the problem(s):**
```
ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestRVPCSite2SiteVpn'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
self.setUp()
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
self.setupContext(ancestor)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
try_run(context, names)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
return func()
File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 835, in setUpClass
cls.template.download(cls.apiclient)
File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
elif 'Downloaded' in template.status:
TypeError: argument of type 'NoneType' is not iterable
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_0H4UMM/results.txt
```
```
ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestVpcRemoteAccessVpn'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
self.setUp()
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
self.setupContext(ancestor)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
try_run(context, names)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
return func()
File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 293, in setUpClass
cls.template.download(cls.apiclient)
File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
elif 'Downloaded' in template.status:
TypeError: argument of type 'NoneType' is not iterable
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_0H4UMM/results.txt
```
```
ERROR: test suite for <class 'integration.smoke.test_vpc_vpn.TestVpcSite2SiteVpn'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 209, in run
self.setUp()
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 292, in setUp
self.setupContext(ancestor)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 315, in setupContext
try_run(context, names)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
return func()
File "/data/git/cs1/cloudstack/test/integration/smoke/test_vpc_vpn.py", line 472, in setUpClass
cls.template.download(cls.apiclient)
File "/usr/lib/python2.7/site-packages/marvin/lib/base.py", line 1350, in download
elif 'Downloaded' in template.status:
TypeError: argument of type 'NoneType' is not iterable
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_0H4UMM/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_32_28_UUXUPD:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_32_28_UUXUPD/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_32_28_UUXUPD/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_19_2016_19_32_28_UUXUPD/runinfo.txt)
**`/tmp/MarvinLogs/test_network_0H4UMM:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_0H4UMM/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_0H4UMM/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_0H4UMM/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_L1B4M6:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_L1B4M6/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_L1B4M6/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_L1B4M6/runinfo.txt)
Uploads will be available until `2016-07-20 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1551
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220379062
This was run prior to your changes. I will need to rerun to incorporate the recent changes...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220378822
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 4h 15m 16s
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_18_2016_19_54_53_7SYUR4:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_18_2016_19_54_53_7SYUR4/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_18_2016_19_54_53_7SYUR4/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/DeployDataCenter__May_18_2016_19_54_53_7SYUR4/runinfo.txt)
**`/tmp/MarvinLogs/test_network_2ZOCMY:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_2ZOCMY/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_2ZOCMY/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_network_2ZOCMY/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_M227NQ:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_M227NQ/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_M227NQ/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1551/tmp/MarvinLogs/test_vpc_routers_M227NQ/runinfo.txt)
Uploads will be available until `2016-07-19 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220380639
@swill thanks, though the latter changes are all packaging related. I've built rpm/deb repository to show packaging works: http://packages.shapeblue.com/cloudstack/custom/master-latest/ (this is master + this PR)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220715917
I am getting it on almost every PR right now (if you notice the last 6 or 7 CI runs), so I will have to review this as it seems to have been introduced recently...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220533786
@swill the error suggests the template response is missing the key 'status' in its json response. It may happen if there has been any change around template (response) code; or if the backend is not setting the status, so effectively gson does not serialize those keys (as they are not set)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: remove unnecessary order b...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220098373
OK, I will run this through my CI right now and if it comes back clean I will get it into this freeze.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: dynamic-roles: packaging improvements
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1551#issuecomment-220262092
Ok, so this PR seems to fix multiple things.
I would LGTM on the packaging fixes as well as the Python part.
The MySQL thing I'm not sure about. I wouldn't say that it is a problem perse, but we all know what one line of code can do (err, I mean break...)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---