merged to master as
[2d9cb407](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/2d9cb407).
Thanks @FileIOUtility!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Closed #439.
--
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-labs/pull/439#event-1738328709
No need for a new PR. Just squash and push+force here.
--
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-labs/pull/439#issuecomment-404577707
Great to see that build, so far I had build failures resulting from different
code quality validations applied for JDK7. I will squash and create a new PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
And build is back to green! @FileIOUtility mind squashing the commits so I
can cleanly merge the PR?
--
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-labs/pull/439#issuecomment-404567165
Everything should be in place now in jclouds-core: rebuild please
--
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-labs/pull/439#issuecomment-404305499
@FileIOUtility pushed 1 commit.
56fb4bc JCLOUDS-1432 - removed unused import
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
FTR: https://github.com/jclouds/jclouds/pull/1224 should fix this build once
merged.
There is one checkstyle violation in this branch though:
```
[WARNING]
src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/ServerApiMockTest.java[31:8]
(imports) UnusedImports: Unused import -
It's because the default build config. No need to close the PR. I'll bump the
source and target versions in the build config and it should be fine.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
build failed again after I committed the suggested change that needs at least
Java 7 to work:
"Undefined reference: void
java.util.ConcurrentModificationException.(String, Throwable)"
Reopened #439.
--
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-labs/pull/439#event-1723645985
Closed #439.
--
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-labs/pull/439#event-1723645736
@FileIOUtility pushed 2 commits.
b13d0f8 JCLOUDS-1432 - review - use JDK7 and ConcurrentModificationException
6e076b3 JCLOUDS-1432 - review - fix test failure message
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
rebuild please
--
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-labs/pull/439#issuecomment-403472574
nacx commented on this pull request.
> @@ -56,7 +57,7 @@ public void testDeployServerReturnsUnexpectedError() throws
> InterruptedException
serverApi().deployServer(ServerApiMockTest.class.getSimpleName(),
"imageId", true, networkInfo,
"administratorPassword");
FYI: build was aborted during "jclouds jdbc core" phase.
--
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-labs/pull/439#issuecomment-403468803
FileIOUtility commented on this pull request.
> @@ -56,7 +57,7 @@ public void testDeployServerReturnsUnexpectedError() throws
> InterruptedException
serverApi().deployServer(ServerApiMockTest.class.getSimpleName(),
"imageId", true, networkInfo,
>EDIT: in Java language level 6, ConcurrentModificationException cannot take
>cause exception, so after all, I don't want to use it.
jclouds needs at least Java 7 to work. We've been trying to keep backward
compatibility in our builds, but we are about to move to Java 8 in the next
major
nacx commented on this pull request.
> @@ -56,7 +57,7 @@ public void testDeployServerReturnsUnexpectedError() throws
> InterruptedException
serverApi().deployServer(ServerApiMockTest.class.getSimpleName(),
"imageId", true, networkInfo,
"administratorPassword");
FileIOUtility commented on this pull request.
> } else if (message.contains("NAME_NOT_UNIQUE")) {
exception = new ResourceAlreadyExistsException(message,
exception);
+} else if (message.contains("UNEXPECTED_ERROR")
+ ||
FileIOUtility commented on this pull request.
revisited error propagation as suggested
--
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-labs/pull/439#pullrequestreview-135072543
@FileIOUtility pushed 1 commit.
5d1305b JCLOUD-51 - review - revisit error propagation,
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
Yes, I will add it to Throwables2.
--
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-labs/pull/439#issuecomment-402714108
>RESOURCE_BUSY - Another operation is in progress on an asset. Could
>alternatively be mapped to ConcurrentModificationException. But it is not on
>the list of exceptions allowed to propagate.
Looks like a good candidate to be added to the list. Wanna add it too and use
that exception here?
I think this is correct.
I will add this as 4th commit (before squashing).
RESOURCE_BUSY - Another operation is in progress on an asset. Could
alternatively be mapped to ConcurrentModificationException. But it is not on
the list of exceptions allowed to propagate.
--
You are receiving this
Thanks, @FileIOUtility! LGTM
>From the discussion about response codes, I've revisited the error handler and
>there are some errors we could consider propagating better:
```java
case 400:
if (message.contains("RESOURCE_NOT_FOUND")
|| message.contains("OPERATION_NOT_SUPPORTED")) {
Is it OK to squash these 3 commits before you merge?
--
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-labs/pull/439#issuecomment-402666510
nacx approved this pull request.
> @@ -121,10 +121,9 @@ public void testListServersWithDatacenterFiltering()
> throws Exception {
return
Uris.uriBuilder("/caas/2.4/6ac1e746-b1ea-4da5-a24e-caf1a978789d/server/server");
}
- public void testListServers_404() throws Exception {
-
FileIOUtility commented on this pull request.
replaced IllegalStateException with IllegalArgumentException
> @@ -66,9 +66,9 @@ public void handleError(HttpCommand command, HttpResponse
> response) {
exception = new AuthorizationException(message, exception);
break;
@FileIOUtility pushed 1 commit.
77e3a33 JCLOUDS-1432 - review - replace IllegalStateException
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
FileIOUtility commented on this pull request.
please see my comments and another commit based on your feedback. thanks
> @@ -68,21 +68,19 @@
@GET
@Path("/server")
@ResponseParser(ParseServers.class)
- @Fallback(Fallbacks.EmptyIterableWithMarkerOnNotFoundOr404.class)
For
@FileIOUtility pushed 1 commit.
10d9e4d JCLOUDS-1432 - review - revert to using
NullOnNotFoundOr404/voidOnNotFoundOr404,
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
nacx commented on this pull request.
Thanks @FileIOUtility!
> @@ -68,21 +68,19 @@
@GET
@Path("/server")
@ResponseParser(ParseServers.class)
- @Fallback(Fallbacks.EmptyIterableWithMarkerOnNotFoundOr404.class)
This is a behavior change since this method will now fail upon 4xx
This is @FileIOUtility first PR for Apache jclouds but he has been working on
several aspects of the project for a while now.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
do not expect 404 when assets not found,
initially done for Server API only
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/439
-- Commit Summary --
* JCLOUDS-1432 - handle RESOURCE_NOT_FOUND, do not expect 404 when assets not
35 matches
Mail list logo
