cgivre commented on a change in pull request #2489:
URL: https://github.com/apache/drill/pull/2489#discussion_r822198917
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
##########
@@ -188,20 +198,13 @@ public WebUserConnection provide() {
}
// User is logged in, get/set the WebSessionResources attribute
- WebSessionResources webSessionResources =
- (WebSessionResources)
session.getAttribute(WebSessionResources.class.getSimpleName());
+ WebSessionResources webSessionResources = (WebSessionResources)
session.getAttribute(WebSessionResources.class.getSimpleName());
if (webSessionResources == null) {
// User is login in for the first time
final DrillbitContext drillbitContext = workManager.getContext();
final DrillConfig config = drillbitContext.getConfig();
- final UserSession drillUserSession = UserSession.Builder.newBuilder()
- .withCredentials(UserBitShared.UserCredentials.newBuilder()
- .setUserName(sessionUserPrincipal.getName())
- .build())
- .withOptionManager(drillbitContext.getOptionManager())
-
.setSupportComplexTypes(config.getBoolean(ExecConstants.CLIENT_SUPPORT_COMPLEX_TYPES))
- .build();
+ final UserSession drillUserSession =
UserSession.Builder.newBuilder().withCredentials(UserBitShared.UserCredentials.newBuilder().setUserName(sessionUserPrincipal.getName()).build()).withOptionManager(drillbitContext.getOptionManager()).setSupportComplexTypes(config.getBoolean(ExecConstants.CLIENT_SUPPORT_COMPLEX_TYPES)).build();
Review comment:
nit: Please revert the line breaks to the original pattern for
readability, here and elsewhere.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRestServer.java
##########
@@ -73,6 +78,11 @@
import java.util.ArrayList;
import java.util.List;
+@OpenAPIDefinition(info = @Info(title = "Apache Drill REST API", description =
"OpenAPI Specification", license = @License(name = "Apache Software Foundation
(ASF)", url = "http://www.apache"; + ".org/licenses/LICENSE-2.0"), contact =
@Contact(name = "Apache Drill", url = "https://drill.apache.org/";)))/*,
+tags = {
+ @Tag(name = "My Tag 1", description = "Tag 1's Description", externalDocs =
@ExternalDocumentation(description = "docs description1")),
+ @Tag(name = "My Tag 2", description = "Tag 2's Description", externalDocs =
@ExternalDocumentation(description = "docs description2"))})*/
Review comment:
Please remove commented out code, here and elsewhere. The line above,
please break that up a bit so that it fits w/o horizontal scroll.
##########
File path: exec/java-exec/pom.xml
##########
@@ -889,11 +914,11 @@
<lookAhead>2</lookAhead>
<isStatic>false</isStatic>
<outputDirectory>${project.build.directory}/generated-sources/</outputDirectory>
-<!--
Review comment:
Do you know why we have this commented out code in this `pom.xml` file?
I'd imagine it is here for a reason, and if so, we could indicate that. If
you have no idea, it's fine and we can just leave it.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -103,6 +109,8 @@ public Response getDrillbitStatus() {
@GET
@Path("/gracePeriod")
@Produces(MediaType.APPLICATION_JSON)
+ @Operation(externalDocs = @ExternalDocumentation(description = "Apache Drill
REST API documentation:", url = "
https://drill.apache.org/docs/stopping-drill/#:~:text=draining%2C%20and%20offline.%0A%20%20%20%20%20--,grace,-%3A%20A%20period%20in";))
Review comment:
Please remove extra spaces after the `url` tag. Here and elsewhere.
Also, is it necessary to URL encode the links, or will Drill figure that out by
itself?
If it is not necessary, I'd recommend rendering them normally, w/o the URL
encoding.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -230,278 +240,300 @@ public ClusterInfo getClusterInfoJSON() {
* @return true if drillbit are the same
*/
private boolean isDrillbitsTheSame(DrillbitEndpoint endpoint1,
DrillbitEndpoint endpoint2) {
- return endpoint1.getAddress().equals(endpoint2.getAddress()) &&
- endpoint1.getControlPort() == endpoint2.getControlPort() &&
- endpoint1.getDataPort() == endpoint2.getDataPort() &&
- endpoint1.getUserPort() == endpoint2.getUserPort();
+ return endpoint1.getAddress().equals(endpoint2.getAddress()) &&
endpoint1.getControlPort() == endpoint2.getControlPort() &&
endpoint1.getDataPort() == endpoint2.getDataPort() && endpoint1.getUserPort()
== endpoint2.getUserPort();
}
private Response setResponse(Map<String, ?> entity) {
- return Response.ok()
- .entity(entity)
- .header("Access-Control-Allow-Origin", "*")
- .header("Access-Control-Allow-Methods", "GET, POST, DELETE, PUT")
- .header("Access-Control-Allow-Credentials","true")
- .allow("OPTIONS").build();
+ return Response.ok().entity(entity).header("Access-Control-Allow-Origin",
"*").header("Access-Control-Allow-Methods", "GET, POST, DELETE,
PUT").header("Access-Control-Allow-Credentials",
"true").allow("OPTIONS").build();
}
private Response shutdown(String resp) throws Exception {
Map<String, String> shutdownInfo = new HashMap<String, String>();
new Thread(new Runnable() {
- @Override
- public void run() {
- try {
- drillbit.close();
- } catch (Exception e) {
- logger.error("Request to shutdown drillbit failed", e);
- }
+ @Override
+ public void run() {
+ try {
+ drillbit.close();
+ } catch (Exception e) {
+ logger.error("Request to shutdown drillbit failed", e);
}
- }).start();
- shutdownInfo.put("response",resp);
+ }
+ }).start();
+ shutdownInfo.put("response", resp);
return setResponse(shutdownInfo);
}
-/**
- * Pretty-printing wrapper class around the ZK-based queue summary.
- */
+ /**
+ * Pretty-printing wrapper class around the ZK-based queue summary.
+ */
-@XmlRootElement
-public static class QueueInfo {
- private final ZKQueueInfo zkQueueInfo;
+ @XmlRootElement
+ public static class QueueInfo {
+ private final ZKQueueInfo zkQueueInfo;
- public static QueueInfo build(ResourceManager rm) {
+ public static QueueInfo build(ResourceManager rm) {
- // Consider queues enabled only if the ZK-based queues are in use.
+ // Consider queues enabled only if the ZK-based queues are in use.
- ThrottledResourceManager throttledRM = null;
- if (rm != null && rm instanceof DynamicResourceManager) {
- DynamicResourceManager dynamicRM = (DynamicResourceManager) rm;
- rm = dynamicRM.activeRM();
+ ThrottledResourceManager throttledRM = null;
+ if (rm != null && rm instanceof DynamicResourceManager) {
Review comment:
Why are these changes here? This looks like this came from another
branch or PR. Can you please rebase on current master and push again?
```
> git checkout master
> git fetch upstream
> git rebase upstream/master
> git checkout <your_branch>
> git rebase master
> git push -f
```
--
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: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
