You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2014/06/30 22:40:53 UTC

[jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

You can merge this Pull Request by running:

  git pull https://github.com/andrewgaul/jclouds jdk7

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/426

-- Commit Summary --

  * Require JDK 7
  * Allow payloads greater than 2 GB

-- File Changes --

    M core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java (11)
    M project/pom.xml (4)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/426.patch
https://github.com/jclouds/jclouds/pull/426.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Gaul <no...@github.com>.
JDK 6 builder fails as expected; need to disable it before committing.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-47586798

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1298](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1298/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-47593102

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Zack Shoylev <no...@github.com>.
> @@ -179,15 +179,10 @@ protected HttpURLConnection convert(HttpRequest request) throws IOException, Int
>              connection.setChunkedStreamingMode(8196);
>              writePayloadToConnection(payload, "streaming", connection);
>           } else {
> -            Long length = checkNotNull(md.getContentLength(), "payload.getContentLength");
> -            // TODO: remove check after moving to JDK 7.
> -            checkArgument(length <= Integer.MAX_VALUE,

Am I reading this right? Should this be <= or > ?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426/files#r15608565

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Phillips <no...@github.com>.
@andrewgaul Anything we're waiting on here..?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51671375

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1415](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1415/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-47591851

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Gaul <no...@github.com>.
Reopened #426.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#event-149490776

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1055](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1055/) ABORTED


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51297474

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Zack Shoylev <no...@github.com>.
> @@ -179,15 +179,10 @@ protected HttpURLConnection convert(HttpRequest request) throws IOException, Int
>              connection.setChunkedStreamingMode(8196);
>              writePayloadToConnection(payload, "streaming", connection);
>           } else {
> -            Long length = checkNotNull(md.getContentLength(), "payload.getContentLength");
> -            // TODO: remove check after moving to JDK 7.
> -            checkArgument(length <= Integer.MAX_VALUE,

Thanks for explaining!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426/files#r15701122

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #944](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/944/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-47585003

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-6-pull-requests #1527](https://jclouds.ci.cloudbees.com/job/jclouds-java-6-pull-requests/1527/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51266846

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Gaul <no...@github.com>.
@jdaggett we cannot merge this until we branch 1.8 off of master.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-50074182

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Jeremy Daggett <no...@github.com>.
@andrewgaul Are we ready to merge this?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-50074052

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-6-pull-requests #1527 FAILURE

Expected.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51297279

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Phillips <no...@github.com>.
> @@ -179,15 +179,10 @@ protected HttpURLConnection convert(HttpRequest request) throws IOException, Int
>              connection.setChunkedStreamingMode(8196);
>              writePayloadToConnection(payload, "streaming", connection);
>           } else {
> -            Long length = checkNotNull(md.getContentLength(), "payload.getContentLength");
> -            // TODO: remove check after moving to JDK 7.
> -            checkArgument(length <= Integer.MAX_VALUE,

I think the idea is that, for Java 6, length must be <= 2147483647, i.e. 2Gb. `checkArgument` verifies that that condition is true, i.e. will only throw an exception if lengh > 2Gb.

That seems correct?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426/files#r15610064

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Gaul <no...@github.com>.
> @@ -179,15 +179,10 @@ protected HttpURLConnection convert(HttpRequest request) throws IOException, Int
>              connection.setChunkedStreamingMode(8196);
>              writePayloadToConnection(payload, "streaming", connection);
>           } else {
> -            Long length = checkNotNull(md.getContentLength(), "payload.getContentLength");
> -            // TODO: remove check after moving to JDK 7.
> -            checkArgument(length <= Integer.MAX_VALUE,

Correct, the previous code enforced a maximum size.  Java 6 only has `HttpURLConnection.setFixedLengthStreamingMode(int)` while Java 7 and onward have `HttpURLConnection.setFixedLengthStreamingMode(long)` as well.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426/files#r15618713

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1457](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1457/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51270738

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Gaul <no...@github.com>.
Closing/reopening to test new CloudBees configuration.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51264936

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Everett Toews <no...@github.com>.
We need to hold off on merging this until we know exactly what version of jclouds will require JDK 7.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-47586419

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #1457 SUCCESS

+1 - good to go for me. Thanks, @andrewgaul!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51300417

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1054](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1054/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51243289

Re: [jclouds] Require JDK 7 and allow payloads greater than 2 GB (#426)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1456](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1456/) ABORTED

[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/426#issuecomment-51263747