You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by afs <gi...@git.apache.org> on 2017/07/27 09:57:11 UTC
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
GitHub user afs opened a pull request:
https://github.com/apache/jena/pull/271
JENA-1378 : Set the accept header in the default use case.
...while still respecting any direct HttpClient setup.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/afs/jena conneg
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/271.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #271
----
commit a89605c150560b42e7d7a46f7d2660678b347ba3
Author: Andy Seaborne <an...@apache.org>
Date: 2017-07-27T09:10:18Z
Expose the setup of the pooling HttpClientBuilder.
For use by RDFParserBuilder.
commit b11e87393e520815c669bd0086434c2960496b78
Author: Andy Seaborne <an...@apache.org>
Date: 2017-07-27T09:12:40Z
JENA-1378: Set the accept header in the default use case.
----
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/271#discussion_r130117735
--- Diff: jena-arq/src/main/java/org/apache/jena/riot/RDFParserBuilder.java ---
@@ -504,8 +504,10 @@ private HttpClient buildHttpClient() {
Header header = new BasicHeader(k, v);
hdrs.add(header);
});
- HttpClient hc = CachingHttpClientBuilder.create().setDefaultHeaders(hdrs).build();
- return hc;
+ HttpClient hc = HttpOp.createPoolingHttpClientBuilder()
--- End diff --
What is required is a builder that is based on another but isolated from it by cloning (or in some way) so that adding new builder settings only changes the clone, not also the original (the current active default in `HttpOp` in this case).`HttpClientBuilder` is a setter only style, you can't get settings from it (builder builder needed ?!). `RDFparserBuilder` has `clone()`.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/271#discussion_r130105145
--- Diff: jena-arq/src/main/java/org/apache/jena/riot/RDFParserBuilder.java ---
@@ -504,8 +504,10 @@ private HttpClient buildHttpClient() {
Header header = new BasicHeader(k, v);
hdrs.add(header);
});
- HttpClient hc = CachingHttpClientBuilder.create().setDefaultHeaders(hdrs).build();
- return hc;
+ HttpClient hc = HttpOp.createPoolingHttpClientBuilder()
--- End diff --
For future refining, my understanding of `HttpClientBuilder` is that it does exactly what you describe above. IOW, you can call `build()` on it and get a client but keep it around to call `build()` on it later for another same-config client, over and over again.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/271#discussion_r130092145
--- Diff: jena-arq/src/main/java/org/apache/jena/riot/RDFParserBuilder.java ---
@@ -504,8 +504,10 @@ private HttpClient buildHttpClient() {
Header header = new BasicHeader(k, v);
hdrs.add(header);
});
- HttpClient hc = CachingHttpClientBuilder.create().setDefaultHeaders(hdrs).build();
- return hc;
+ HttpClient hc = HttpOp.createPoolingHttpClientBuilder()
--- End diff --
A possibility though I like that HttpOp is neutral as to its usage - it is not RDF-specific and the default headers are none. There are different headers for graph, datasets and result sets. Responsibility for the customization is in the RDFParser which here (this is the case of the app making detailed settings) captures those choices for a reusable parser process.
Adding to the complexity of HttpOp.createPoolingHttpClient seems a slippery slope that goes against the builder pattern (IMO). It would be easier if there were an HttpComponents way to say "make a builder that would produce this HttpClient" capturing the internal settings details of an HttpClient.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/271
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/271#discussion_r130093724
--- Diff: jena-arq/src/main/java/org/apache/jena/riot/RDFParserBuilder.java ---
@@ -504,8 +504,10 @@ private HttpClient buildHttpClient() {
Header header = new BasicHeader(k, v);
hdrs.add(header);
});
- HttpClient hc = CachingHttpClientBuilder.create().setDefaultHeaders(hdrs).build();
- return hc;
+ HttpClient hc = HttpOp.createPoolingHttpClientBuilder()
--- End diff --
Merged, which fixes the bug as reported. That does not preclude refining it some more.
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #271: JENA-1378 : Set the accept header in the default use...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/271#discussion_r129900223
--- Diff: jena-arq/src/main/java/org/apache/jena/riot/RDFParserBuilder.java ---
@@ -504,8 +504,10 @@ private HttpClient buildHttpClient() {
Header header = new BasicHeader(k, v);
hdrs.add(header);
});
- HttpClient hc = CachingHttpClientBuilder.create().setDefaultHeaders(hdrs).build();
- return hc;
+ HttpClient hc = HttpOp.createPoolingHttpClientBuilder()
--- End diff --
Should we maybe just put the default headers into the clients built by `HttpOp` as options (e.g. `HttpOp.createPoolingHttpClientBuilder(boolean useDefaultAcceptHeaderForRdf)` or something like that?
---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---