Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-216440946
Thanks @rhtyd. I will do a CI run on master before I merge anything else.
I will also pull in a couple people who I know have worked on this code to
review your upd
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1321#discussion_r61833885
--- Diff:
engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---
@@ -505,4 +513,24 @@ public void testSendStopWithNullAnswer() throw
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1321#discussion_r61833399
--- Diff:
engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---
@@ -505,4 +513,24 @@ public void testSendStopWithNullAnswer() throw
Github user rhtyd commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1321#discussion_r61833186
--- Diff:
engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---
@@ -505,4 +513,24 @@ public void testSendStopWithNullAnswer() throw
Github user swill commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1321#discussion_r61808445
--- Diff:
engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---
@@ -505,4 +513,24 @@ public void testSendStopWithNullAnswer() throw
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1321
---
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
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-216320768
gogogo
---
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 featu
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-216302010
All tests have passed on this and we have the required number of code
reviews. There are no merge conflicts, so I think we should be fine to merge
this PR as is. An
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-216220379
@nitin-maharana thanks, please rebase the branch again.
LGTM (without testing)
tag:easypr
---
If your project is set up for it, you can reply to thi
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-212898459
@koushik-das : I also think the same. Yes, that would be better. I will
raise another PR with removing the existing implementation. pinging @agneya2001
@remi
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-212868899
@nitin-maharana I checked the discussions on #823 as well. Looks like it is
best to remove all tag checks for selecting offering. Also there is no usage
impact
Github user bvbharatk commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-201764810
_Link to logs Folder (search by build_no):_
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
### ACS CI BVT Run
**Sumarry:**
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-175690290
LGTM, should we merge this?
---
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 no
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-172908360
@DaanHoogland This PR is better than the current situation and would
improve even more if the whole check would disappear. If the PR is changed,
I'll happily ru
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-172911093
ok, clear
---
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 fe
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-172903549
@remibergsma I am puzzled. You don't agree with the contents of the PR but
you did run the tests? or did I misread your comment? I though you and
@agneya2001 d
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-172892988
LGTM based on these tests:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a
tags=advanced,required_hardware=true \
component/test_
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-171885529
+1 for removing the check. I now hack the db if I want to do this and that
shouldn't be necessary.
---
If your project is set up for it, you can reply to this
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-171865987
tag should not be a reason for incompatibility. that is you can mover from
a service offering that is similar but with any other set of tags. we should
just do
Github user nitin-maharana closed the pull request at:
https://github.com/apache/cloudstack/pull/823
---
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 fea
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-169935147
Closing this PR as made a new PR #1321 (Against 4.7 which will be merged in
master later).
---
If your project is set up for it, you can reply to this email
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/1321#issuecomment-169934934
Reference PR #823 (Against master). This PR (against 4.7) contains the same
code change.
---
If your project is set up for it, you can reply to this email a
GitHub user nitin-maharana opened a pull request:
https://github.com/apache/cloudstack/pull/1321
CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged
offerings when called with VM id
When calling listServiceOfferings with VM id as parameter. It is returning
incomp
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-165325933
@nitin-maharana @koushik-das @kishankavala @bhaisaab @jburwell
@DaanHoogland @remibergsma Can we have a discussion on: Why do we need any kind
of restriction on t
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-165102057
@nitin-maharana i am wondering why should change in service offering have
any restrictions on tags ? @kishankavala any usage implications.
---
If your project i
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-165074680
@agneya2001 : If the new service offering wants in change of deployment
then it should be fine. But the change affects after reboot only. Thanks.
---
If your
Github user agneya2001 commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-163834017
@nitin-maharana The current way was to move the service offering to a
similar or a more flexible one in terms of deployment. While this change will
make the new s
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-163550095
@DaanHoogland : No, it needn't not be an exact match but the new one should
be a superset of the current one. According to your scenario, as the new one
suppo
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-163212979
@nitin-maharana : Then you are saying it must be an exact match. Now a
compute offering that had (x,y) can be replaced with one that has (x,y,z) while
the VM de
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-162947373
The functionality purely depends on the meaning of compatibility. Here is
what I understood about the compatibility. For example, if our current compute
offer
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-162454833
The code looks good but I am wondering about the functionality. The prior
situation was that the new offering should have a subset of the old one. Now it
has to
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-162346076
Pinging @DaanHoogland to review.
---
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
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-153002143
LGTM, based on a set of tests that I run on this branch (which I rebased
myself first):
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-152453158
I tested this with this scenario.
There are three service offerings.
{
"listserviceofferingsresponse": {
"count": 3,
Github user nitin-maharana commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-144333001
Hi @remibergsma,
I rebased my commit against the current master.
I added unit test for the change.
Thanks,
Nitin
---
If your project is
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/823#issuecomment-143590275
@nitin-maharana Thanks! Can you please rebase against current master?
How did you test this?
---
If your project is set up for it, you can reply to this
GitHub user nitin-maharana opened a pull request:
https://github.com/apache/cloudstack/pull/823
CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged
offerings when called with VM id
When calling listServiceOfferings with VM id as parameter. It is returning
incompa
37 matches
Mail list logo
