cnauroth commented on code in PR #7453:
URL: https://github.com/apache/hadoop/pull/7453#discussion_r1980051217
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice-hbase-tests/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/AbstractTimelineReaderHBaseTestBase.java:
##########
@@ -132,39 +135,38 @@ protected ClientResponse getResponse(Client client, URI
uri)
return resp;
}
- protected void verifyHttpResponse(Client client, URI uri, Status status) {
- ClientResponse resp =
- client.resource(uri).accept(MediaType.APPLICATION_JSON)
- .type(MediaType.APPLICATION_JSON).get(ClientResponse.class);
+ protected void verifyHttpResponse(Client client, URI uri, Response.Status
status) {
+ Response resp =
client.target(uri).request(MediaType.APPLICATION_JSON).get();
assertNotNull(resp);
assertTrue("Response from server should have been " + status,
resp.getStatusInfo().getStatusCode() == status.getStatusCode());
- System.out.println("Response is: " + resp.getEntity(String.class));
+ System.out.println("Response is: " + resp.readEntity(String.class));
}
protected List<FlowActivityEntity> verifyFlowEntites(Client client, URI uri,
int noOfEntities) throws Exception {
- ClientResponse resp = getResponse(client, uri);
+ Response resp = getResponse(client, uri);
List<FlowActivityEntity> entities =
- resp.getEntity(new GenericType<List<FlowActivityEntity>>() {
+ resp.readEntity(new GenericType<List<FlowActivityEntity>>() {
});
assertNotNull(entities);
assertEquals(noOfEntities, entities.size());
return entities;
}
- protected static class DummyURLConnectionFactory
- implements HttpURLConnectionFactory {
-
- @Override
- public HttpURLConnection getHttpURLConnection(final URL url)
- throws IOException {
- try {
- return (HttpURLConnection) url.openConnection();
- } catch (UndeclaredThrowableException e) {
- throw new IOException(e.getCause());
- }
- }
+ @VisibleForTesting
+ protected HttpUrlConnectorProvider getHttpURLConnectionFactory() {
+ return new HttpUrlConnectorProvider().connectionFactory(
+ url -> {
+ HttpURLConnection conn;
+ try {
+ HttpURLConnection.setFollowRedirects(false);
Review Comment:
Why was it necessary to disable following redirects? This change is applied
globally for the whole JVM. I wonder if it could have unintended side effects
for other areas that expect to follow redirects (which is the default behavior).
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/timelineservice/TimelineEntity.java:
##########
@@ -150,7 +152,7 @@ public boolean equals(Object obj) {
private NavigableSet<TimelineEvent> events = new TreeSet<>();
private HashMap<String, Set<String>> isRelatedToEntities = new HashMap<>();
private HashMap<String, Set<String>> relatesToEntities = new HashMap<>();
- private Long createdTime;
+ private Long createdTime = Instant.EPOCH.toEpochMilli();
Review Comment:
Is this potentially a backward-incompatible change? Could we start returning
"1970" where previously the value was null/missing?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-timelineservice-hbase-tests/src/test/java/org/apache/hadoop/yarn/server/timelineservice/reader/AbstractTimelineReaderHBaseTestBase.java:
##########
@@ -109,19 +106,25 @@ protected void addFilters(Configuration conf) {
}
protected Client createClient() {
- ClientConfig cfg = new DefaultClientConfig();
- cfg.getClasses().add(YarnJacksonJaxbJsonProvider.class);
- return new Client(
- new URLConnectionClientHandler(new DummyURLConnectionFactory()), cfg);
+ final ClientConfig cc = new ClientConfig();
+ cc.connectorProvider(getHttpURLConnectionFactory());
+ return ClientBuilder.newClient(cc)
+ .register(TimelineEntityReader.class)
Review Comment:
I'm not familiar with differences in the new Jersey version, but it seems
like the prior code was much more driven by introspection and annotations
instead of needing to register a lot of custom readers. Is there any way to
make it more like the prior code? I'm concerned about risk of
backward-incompatible changes in the manual approach and potential error-prone
evolution of the code if fields are changed in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
