-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-18306: Warnings should not be shown on cli console when linux user not present on client #4474
Conversation
…user not present on client
@@ -215,7 +215,13 @@ private Set<String> getUnixGroups(String user) throws IOException { | |||
groups = resolvePartialGroupNames(user, e.getMessage(), | |||
executor.getOutput()); | |||
} catch (PartialGroupNameException pge) { | |||
LOG.warn("unable to return groups for user {}", user, pge); | |||
if(LOG.isDebugEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just extract this into String msg = ""unable to return groups for user {}" and use this msg to make log line looks clean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smengcl do you think this is fine? or we don't need this in even in Warn?
we were hitting this log message. This current patch voids the full trace in logs when debug is not enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes IMO we don't need the warn.
@swamirishi Just be sure to double check the debug log can actually be enabled for example by setting org.apache.hadoop.security.ShellBasedUnixGroupsMapping=DEBUG
in log4j.properties
or JVM -D
param.
nit: checkstyle missing space after if
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@swamirishi could you please fix the @smengcl comments above? you need to format line number 218
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is stale version. Thanks
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
The -1 is due to no tests added. -1 overall [2022-06-23T22:22:07.359Z] [2022-06-23T22:22:07.359Z] | Vote | Subsystem | Runtime | Comment [2022-06-23T22:22:07.359Z] ============================================================================ [2022-06-23T22:22:07.360Z] | 0 | reexec | 0m 55s | Docker mode activated. [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | | | | Prechecks [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | +1 | dupname | 0m 0s | No case conflicting files found. [2022-06-23T22:22:07.360Z] | 0 | codespell | 0m 0s | codespell was not available. [2022-06-23T22:22:07.360Z] | 0 | detsecrets | 0m 0s | detect-secrets was not available. [2022-06-23T22:22:07.360Z] | +1 | @author | 0m 0s | The patch does not contain any @author [2022-06-23T22:22:07.360Z] | | | | tags. [2022-06-23T22:22:07.360Z] | -1 | test4tests | 0m 0s | The patch doesn't appear to include [2022-06-23T22:22:07.360Z] | | | | any new or modified tests. Please [2022-06-23T22:22:07.360Z] | | | | justify why no new tests are needed for [2022-06-23T22:22:07.360Z] | | | | this patch. Also please list what [2022-06-23T22:22:07.360Z] | | | | manual steps were performed to verify [2022-06-23T22:22:07.360Z] | | | | this patch. [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | | | | trunk Compile Tests [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | +1 | mvninstall | 40m 24s | trunk passed [2022-06-23T22:22:07.360Z] | +1 | compile | 25m 10s | trunk passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 [2022-06-23T22:22:07.360Z] | | | | [2022-06-23T22:22:07.360Z] | +1 | compile | 21m 37s | trunk passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-1.8.0_312-8u312-b07-0ubuntu1~20.04 [2022-06-23T22:22:07.360Z] | | | | -b07 [2022-06-23T22:22:07.360Z] | +1 | checkstyle | 1m 31s | trunk passed [2022-06-23T22:22:07.360Z] | +1 | mvnsite | 1m 58s | trunk passed [2022-06-23T22:22:07.360Z] | +1 | javadoc | 1m 31s | trunk passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 [2022-06-23T22:22:07.360Z] | | | | [2022-06-23T22:22:07.360Z] | +1 | javadoc | 1m 4s | trunk passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-1.8.0_312-8u312-b07-0ubuntu1~20.04 [2022-06-23T22:22:07.360Z] | | | | -b07 [2022-06-23T22:22:07.360Z] | +1 | spotbugs | 3m 6s | trunk passed [2022-06-23T22:22:07.360Z] | +1 | shadedclient | 25m 52s | branch has no errors when building and [2022-06-23T22:22:07.360Z] | | | | testing our client artifacts. [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | | | | Patch Compile Tests [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | +1 | mvninstall | 1m 6s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | compile | 24m 13s | the patch passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 [2022-06-23T22:22:07.360Z] | | | | [2022-06-23T22:22:07.360Z] | +1 | javac | 24m 13s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | compile | 21m 45s | the patch passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-1.8.0_312-8u312-b07-0ubuntu1~20.04 [2022-06-23T22:22:07.360Z] | | | | -b07 [2022-06-23T22:22:07.360Z] | +1 | javac | 21m 45s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | blanks | 0m 0s | The patch has no blanks issues. [2022-06-23T22:22:07.360Z] | +1 | checkstyle | 1m 24s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | mvnsite | 1m 56s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | javadoc | 1m 22s | the patch passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 [2022-06-23T22:22:07.360Z] | | | | [2022-06-23T22:22:07.360Z] | +1 | javadoc | 1m 4s | the patch passed with JDK Private [2022-06-23T22:22:07.360Z] | | | | Build-1.8.0_312-8u312-b07-0ubuntu1~20.04 [2022-06-23T22:22:07.360Z] | | | | -b07 [2022-06-23T22:22:07.360Z] | +1 | spotbugs | 3m 3s | the patch passed [2022-06-23T22:22:07.360Z] | +1 | shadedclient | 25m 37s | patch has no errors when building and [2022-06-23T22:22:07.360Z] | | | | testing our client artifacts. [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | | | | Other Tests [2022-06-23T22:22:07.360Z] +--------------------------------------------------------------------------- [2022-06-23T22:22:07.360Z] | +1 | unit | 18m 17s | hadoop-common in the patch passed. [2022-06-23T22:22:07.360Z] | +1 | asflicense | 1m 16s | The patch does not generate ASF [2022-06-23T22:22:07.360Z] | | | | License warnings. [2022-06-23T22:22:07.360Z] | | | 224m 51s | Since this is only log message level change, I am approving this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
…user not present on client (apache#4474). Contributed by swamirishi.
… when linux user not present on client (apache#4474). Contributed by swamirishi. Change-Id: Ia512ab790f501fb2b7f52773c2d4a8d8692d9d1d
Description of PR
Warnings should not be shown on cli console when linux user not present on client
How was this patch tested?
Logging Changes