cgivre commented on a change in pull request #2489:
URL: https://github.com/apache/drill/pull/2489#discussion_r823809822
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -181,19 +194,15 @@ public ClusterInfo getClusterInfoJSON() {
final String currentVersion = currentDrillbit.getVersion();
final DrillConfig config = dbContext.getConfig();
- final boolean userEncryptionEnabled =
- config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED) ||
- config .getBoolean(ExecConstants.USER_SSL_ENABLED);
+ final boolean userEncryptionEnabled =
config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED) ||
config.getBoolean(ExecConstants.USER_SSL_ENABLED);
final boolean bitEncryptionEnabled =
config.getBoolean(ExecConstants.BIT_ENCRYPTION_SASL_ENABLED);
OptionManager optionManager = work.getContext().getOptionManager();
final boolean isUserLoggedIn = AuthDynamicFeature.isUserLoggedIn(sc);
- final boolean shouldShowAdminInfo = isUserLoggedIn &&
((DrillUserPrincipal)sc.getUserPrincipal()).isAdminUser();
+ final boolean shouldShowAdminInfo = isUserLoggedIn &&
((DrillUserPrincipal) sc.getUserPrincipal()).isAdminUser();
Review comment:
nit: remove space.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -181,19 +194,15 @@ public ClusterInfo getClusterInfoJSON() {
final String currentVersion = currentDrillbit.getVersion();
final DrillConfig config = dbContext.getConfig();
- final boolean userEncryptionEnabled =
- config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED) ||
- config .getBoolean(ExecConstants.USER_SSL_ENABLED);
+ final boolean userEncryptionEnabled =
config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED) ||
config.getBoolean(ExecConstants.USER_SSL_ENABLED);
final boolean bitEncryptionEnabled =
config.getBoolean(ExecConstants.BIT_ENCRYPTION_SASL_ENABLED);
OptionManager optionManager = work.getContext().getOptionManager();
final boolean isUserLoggedIn = AuthDynamicFeature.isUserLoggedIn(sc);
- final boolean shouldShowAdminInfo = isUserLoggedIn &&
((DrillUserPrincipal)sc.getUserPrincipal()).isAdminUser();
+ final boolean shouldShowAdminInfo = isUserLoggedIn &&
((DrillUserPrincipal) sc.getUserPrincipal()).isAdminUser();
for (DrillbitEndpoint endpoint : work.getContext().getAvailableBits()) {
- final DrillbitInfo drillbit = new DrillbitInfo(endpoint,
- isDrillbitsTheSame(currentDrillbit, endpoint),
- currentVersion.equals(endpoint.getVersion()));
+ final DrillbitInfo drillbit = new DrillbitInfo(endpoint,
isDrillbitsTheSame(currentDrillbit, endpoint),
currentVersion.equals(endpoint.getVersion()));
Review comment:
Nit: Please revert spacing changes. Here and elsewhere.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -208,18 +217,12 @@ public ClusterInfo getClusterInfoJSON() {
String adminUsers =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
String adminUserGroups =
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager);
- logger.debug("Admin info: user: " + adminUsers + " user group: " +
adminUserGroups +
- " userLoggedIn " + isUserLoggedIn + " shouldShowAdminInfo: " +
shouldShowAdminInfo);
+ logger.debug("Admin info: user: " + adminUsers + " user group: " +
adminUserGroups + " userLoggedIn " + isUserLoggedIn + " shouldShowAdminInfo: "
+ shouldShowAdminInfo);
Review comment:
Please format log messages as shown below: (I didn't do the whole line
but you get the idea.)
```
logger.debug("Admin info: {} user group: {}", adminUsers, adminUserGroups);
```
##########
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:
I think a link the general page is fine. If there is an internal
reference IE:
https://somepage.com#part2
Definitely include that. But I'd remove the URL encoding if it isn't
necessary.
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
##########
@@ -208,18 +217,12 @@ public ClusterInfo getClusterInfoJSON() {
String adminUsers =
ExecConstants.ADMIN_USERS_VALIDATOR.getAdminUsers(optionManager);
String adminUserGroups =
ExecConstants.ADMIN_USER_GROUPS_VALIDATOR.getAdminUserGroups(optionManager);
- logger.debug("Admin info: user: " + adminUsers + " user group: " +
adminUserGroups +
- " userLoggedIn " + isUserLoggedIn + " shouldShowAdminInfo: " +
shouldShowAdminInfo);
+ logger.debug("Admin info: user: " + adminUsers + " user group: " +
adminUserGroups + " userLoggedIn " + isUserLoggedIn + " shouldShowAdminInfo: "
+ shouldShowAdminInfo);
- return new ClusterInfo(drillbits, currentVersion, mismatchedVersions,
- userEncryptionEnabled, bitEncryptionEnabled, shouldShowAdminInfo,
- QueueInfo.build(dbContext.getResourceManager()),
- processUser, processUserGroups, adminUsers, adminUserGroups,
authEnabled.get());
+ return new ClusterInfo(drillbits, currentVersion, mismatchedVersions,
userEncryptionEnabled, bitEncryptionEnabled, shouldShowAdminInfo,
QueueInfo.build(dbContext.getResourceManager()), processUser,
processUserGroups, adminUsers, adminUserGroups, authEnabled.get());
Review comment:
Please revert spacing changes, here and elsewheree.
--
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
