> I think this change is not safe
Thanks for catching this, @nacx! Are we perhaps missing a unit test somewhere
to ensure the cleanup method behaves as you described?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Well, i can confirm it fixes the failing tests :clap:
I'll run the full suite of LiveTests to see if i find more problems
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Checked again:
The problem happens when trying to security groups from nodes not created by
jclouds and therefore, not respecting naming convention (using Upper case
breaks
Ok! I'll take a look at it again.
When i tried it it was trying to remove a security group already existing in
the user subscription, maybe the problem is there
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
[
https://issues.apache.org/jira/browse/JCLOUDS-1294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignasi Barrera resolved JCLOUDS-1294.
-
Resolution: Fixed
Fix Version/s: 2.1.1
2.2.0
> Azure ARM
Closed #1203.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1203#event-1609392221
[
https://issues.apache.org/jira/browse/JCLOUDS-1294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16463589#comment-16463589
]
ASF subversion and git services commented on JCLOUDS-1294:
--
Commit
Merged to
[master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/b29f716a) and
[2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/b144d9f4).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
[
https://issues.apache.org/jira/browse/JCLOUDS-1294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16463588#comment-16463588
]
ASF subversion and git services commented on JCLOUDS-1294:
--
Commit
Pushed to
[master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/7c592703) and
[2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/a32690a2). Thanks
@danielestevez!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it
Closed #1201.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1201#event-1609362060
nacx approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1201#pullrequestreview-117529420
andreaturli approved this pull request.
+1
thanks
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1203#pullrequestreview-117528611
@nacx pushed 1 commit.
72b76a4 Address review comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1203/files/c0db82e6796fde2a2bcdd0e585b3bf9bec10c643..72b76a4e7daee5974e29fb89e1b7948ac3712acd
andreaturli approved this pull request.
looks good to me and very useful! Thanks!
`AzureRetryableErrorHandlerTest` suite is ok too
Please update https://issues.apache.org/jira/browse/JCLOUDS-1294 accordingly.
> + if (response.getStatusCode() != 429 || isRateLimitError(response)) {
+
I think this change is not safe. The group name the method [gets passed
in](https://github.com/jclouds/jclouds/blob/master/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeService.java#L120)
is **not** the name of a security group. It is the name of the
https://issues.apache.org/jira/browse/JCLOUDS-1294
This adds a handler for Azure `RetryableError` codes. Note that there is a
second commit in the PR with some small cleanup that is not relevant to this PR.
@danielestevez Could you give this branch a try?
You can view, comment on, or merge this
17 matches
Mail list logo
