[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-05-03 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-386446908
 
 
   @rhtyd I have been thinking about this concurrency problem and it can be 
problematic for us. I will address it. I will take your design for the 
updateRole in consideration. 
   
   I will not be able to address it right now though; I will be able to work on 
it probably in the next month or so. This PR was only merged to 4.12+, so we 
are safe here (4.11 will have the problem I solved here + the concurrency one 
though).


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-05-03 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-386326837
 
 
   Ok, I see what you did there.
   
   Our goal was not to tackle concurrency problems with that new API method. We 
have not had these problems so far. The idea was only to fix the drag and drop 
sorting/limitations. I can put this in my backlog, and look for a way to tackle 
it as well. However, I would prefer a new method to maintain backward 
compatibility of that “update” method.
   
   BTW: can I open a transaction in the service (manager) layer? I really do 
not like the idea of putting logic in DAOs.  If we were using spring to manage 
DAOs and transaction we would have `@Transactional` annotation. I see that in 
ACS some parts of the code use `@DB`, will it open a transaction for methods 
annotated with it?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-05-03 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-386267109
 
 
   I did not understand what you mean.
   
   Are you saying we keep in javascript a list with the state of the original 
ACL list, and then when we change, we create another list, we then send these 
two lists to the server so the server can move the ACLs that changed places?
   
   So, you would like to add a new parameters to the update method? Is that it?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-05-03 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-386267109
 
 
   I did not understand what you mean.
   
   Are you saying we keep in javascript a list with the state of the original 
ACL list, and then when we change, we send these two lists to the server so the 
server can move the ACLs that changed places?
   
   So, you would like to add a new parameters to the update method? Is that it?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-04-10 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-380148227
 
 
   @nitin-maharana do you approve this PR?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-04-10 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-380148227
 
 
   @nitin-maharana did you approve this PR?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-04-05 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-378890282
 
 
   I looked into it. The test failed because the uptime of the VR is less than 
3 minutes? 
   This looks like a "test" prone to fail due to environment differences. Shall 
we execute the tests again?
   
   ```
   Traceback (most recent call last):
 File "/marvin/tests/smoke/test_routers.py", line 526, in 
test_04_restart_network_wo_cleanup
   "Check uptime is less than 3 mins or not"
   AssertionError: Check uptime is less than 3 mins or not
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-04-04 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-378729340
 
 
   @borisstoyanov these errors seem to be recurrent. Can I consider that 
everything is ok? or should I investigate deeper the failures reported?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-03-29 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-377206430
 
 
   @DaanHoogland here I am seeing that 
`test_04_rvpc_network_garbage_collector_nics` test that is failing in one of my 
other PRs. Interesting enough, here there are other failures, but I did not 
change code that was already in ACS. Everything I introduce here is new, so 
this changes should not affect anything else.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-03-28 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-376940534
 
 
   @nitin-maharana thanks for the review. Do you approve the PR?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)

2018-03-26 Thread GitBox
rafaelweingartner commented on issue #2511: [CLOUDSTACK-10344] bug when moving 
ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#issuecomment-376148484
 
 
   @nitin-maharana thanks for your review.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services