You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "ramitg254 (via GitHub)" <gi...@apache.org> on 2025/04/19 07:37:39 UTC
[PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
ramitg254 opened a new pull request, #5776:
URL: https://github.com/apache/hive/pull/5776
β¦gin 4.8.6.6
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Upgrade spotbugs-maven-plugin to 4.8.6.6
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Upgraded plugin introduces new rules to identify potential bugs.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Locally, running spotbugs check and fixing the bugs, followed by running basic manual test on hive by building locally and running queries.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051964109
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,6 +211,12 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance(Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
+ @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "intended")
Review Comment:
done returned the fileStatus itself and suppressed the warning
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2817712370
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052366552
##########
shims/common/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.util;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.CLASS)
+public @interface SuppressFBWarnings {
Review Comment:
yeah it can be removed , done π
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2822738730
@ramitg254 Thanks for resolving the bunch of new violations!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin merged PR #5776:
URL: https://github.com/apache/hive/pull/5776
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2818826576
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051964109
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,6 +211,12 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance(Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
+ @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "intended")
Review Comment:
done returned the fileStatus itself and suppressed the warning
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052217942
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -37,10 +37,7 @@
import java.io.IOException;
import java.net.URI;
import java.security.PrivilegedExceptionAction;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
Review Comment:
changed to individual imports
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052366274
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin;
+ KeyProvider keyP;
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
Review Comment:
done updated π
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051958832
##########
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java:
##########
@@ -185,10 +185,27 @@ public float getProgress() throws IOException {
* A generic RecordReader that can hand out different recordReaders
* for each chunk in the CombineFileSplit.
*/
- public CombineFileRecordReader(JobConf job, CombineFileSplit split,
- Reporter reporter,
- Class<RecordReader<K, V>> rrClass)
- throws IOException {
+ public static <K,V> CombineFileRecordReader<K,V> createInstance(JobConf job, CombineFileSplit split,
+ Reporter reporter,
+ Class<RecordReader<K, V>> rrClass) throws IOException{
Review Comment:
done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051965701
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,6 +211,12 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance(Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
+ @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "intended")
public FileStatus getFileStatus() {
return fileStatus;
}
Review Comment:
done returned fileStatus itself and suppressed the warning
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051960653
##########
shims/common/pom.xml:
##########
@@ -106,6 +106,10 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
+ <artifactId>hive-storage-api</artifactId>
+ </dependency>
Review Comment:
this i added because there was no dependency for suppressing warning annotation for hive-shims-common so i used one from hive-storage-api because other ones were leading to cyclic dependency
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051463219
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
Review Comment:
done
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
Review Comment:
done
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
Review Comment:
done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052360973
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin;
+ KeyProvider keyP;
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
Review Comment:
I didn't see that, thanks for pointing it out , I'll update
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051459153
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
+ return new FileStatus(other.getLen(), other.isDirectory(), other.getReplication(), other.getBlockSize(), other.getModificationTime(), other.getAccessTime(), other.getPermission(), other.getOwner(), other.getGroup(), symlink, other.getPath());
Review Comment:
I'll break it into 2-3 lines
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051959223
##########
shims/common/src/main/java/org/apache/hadoop/hive/shims/ShimLoader.java:
##########
@@ -87,7 +87,9 @@ public static HadoopShims getHadoopShims() {
}
}
}
- return hadoopShims;
+ synchronized (ShimLoader.class) {
+ return loadShims(HADOOP_SHIM_CLASSES, HadoopShims.class);
+ }
Review Comment:
done returned `hadoopShims` and suppressed the warning
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052360973
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin;
+ KeyProvider keyP;
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
Review Comment:
I didn't see that, I'll update
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052351667
##########
shims/common/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.util;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.CLASS)
+public @interface SuppressFBWarnings {
Review Comment:
Can we remove a copy from shims/0.23?
https://github.com/apache/hive/blob/master/shims/0.23/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java
I assume shims/0.23 inherits shims/common here. If my assumption is wrong, we can keep duplicates at this point.
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin;
+ KeyProvider keyP;
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
Review Comment:
```suggestion
HdfsAdmin hadmin = new HdfsAdmin(uri, conf);
KeyProvider keyP = hadmin.getKeyProvider();
```
Thanks for the update! In this case, we may not have to separate assignments from declarations.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051458644
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
+ return new FileStatus(other.getLen(), other.isDirectory(), other.getReplication(), other.getBlockSize(), other.getModificationTime(), other.getAccessTime(), other.getPermission(), other.getOwner(), other.getGroup(), symlink, other.getPath());
Review Comment:
i used this parameterised constructor to prevent `"EI_EXPOSE_REP"`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2816703579
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051686241
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -774,26 +772,28 @@ public FutureDataInputStreamBuilder openFile(Path path) throws IOException, Unsu
}
@Override
- public RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f)
- throws FileNotFoundException, IOException {
- return new RemoteIterator<LocatedFileStatus>() {
- private final RemoteIterator<LocatedFileStatus> stats =
- ProxyFileSystem23.super.listLocatedStatus(
- ProxyFileSystem23.super.swizzleParamPath(f));
-
- @Override
- public boolean hasNext() throws IOException {
- return stats.hasNext();
- }
+ public RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f) throws FileNotFoundException, IOException {
+ try {
+ final RemoteIterator<LocatedFileStatus> remoteIterator =
+ ProxyFileSystem23.super.listLocatedStatus(ProxyFileSystem23.super.swizzleParamPath(f));
+ return new RemoteIterator<LocatedFileStatus>() {
+ private final RemoteIterator<LocatedFileStatus> stats = remoteIterator;
+
+ @Override
+ public boolean hasNext() throws IOException {
+ return stats.hasNext();
+ }
- @Override
- public LocatedFileStatus next() throws IOException {
- LocatedFileStatus result = stats.next();
- return new LocatedFileStatus(
- ProxyFileSystem23.super.swizzleFileStatus(result, false),
- result.getBlockLocations());
- }
- };
+ @Override
+ public LocatedFileStatus next() throws IOException {
+ LocatedFileStatus result = stats.next();
+ return new LocatedFileStatus(ProxyFileSystem23.super.swizzleFileStatus(result, false),
+ result.getBlockLocations());
+ }
+ };
+ } catch (IOException e) {
Review Comment:
Can we remove this try-catch because this method throws an IOException?
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1335,23 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin = null;
+ KeyProvider keyP = null;
+ try {
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
+ } catch (IOException e) {
Review Comment:
Can we remove this try-catch?
##########
shims/common/src/main/java/org/apache/hadoop/hive/shims/ShimLoader.java:
##########
@@ -87,7 +87,9 @@ public static HadoopShims getHadoopShims() {
}
}
}
- return hadoopShims;
+ synchronized (ShimLoader.class) {
+ return loadShims(HADOOP_SHIM_CLASSES, HadoopShims.class);
+ }
Review Comment:
I think we intentionally return a cached `hadoopShims` for performance. I guess we need to mark it as a false positive.
- https://github.com/spotbugs/spotbugs/issues/1747
- https://github.com/apache/hbase/pull/4787/files
##########
shims/common/src/main/java/org/apache/hadoop/fs/ProxyFileSystem.java:
##########
@@ -207,21 +199,25 @@ public FileStatus[] listStatus(Path f) throws IOException {
@Override //ref. HADOOP-12502
public RemoteIterator<FileStatus> listStatusIterator(Path f) throws IOException {
- return new RemoteIterator<FileStatus>() {
- private final RemoteIterator<FileStatus> orig =
- ProxyFileSystem.super.listStatusIterator(swizzleParamPath(f));
-
- @Override
- public boolean hasNext() throws IOException {
- return orig.hasNext();
- }
-
- @Override
- public FileStatus next() throws IOException {
- FileStatus ret = orig.next();
- return swizzleFileStatus(ret, false);
- }
- };
+ try {
+ final RemoteIterator<FileStatus> remoteIterator = ProxyFileSystem.super.listStatusIterator(swizzleParamPath(f));
+ return new RemoteIterator<FileStatus>() {
+ private final RemoteIterator<FileStatus> orig = remoteIterator;
+
+ @Override
+ public boolean hasNext() throws IOException {
+ return orig.hasNext();
+ }
+
+ @Override
+ public FileStatus next() throws IOException {
+ FileStatus ret = orig.next();
+ return swizzleFileStatus(ret, false);
+ }
+ };
+ } catch (IOException e) {
Review Comment:
Can we remove this try-catch?
##########
shims/common/pom.xml:
##########
@@ -106,6 +106,10 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
+ <artifactId>hive-storage-api</artifactId>
+ </dependency>
Review Comment:
Do we need this one?
##########
shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java:
##########
@@ -185,10 +185,27 @@ public float getProgress() throws IOException {
* A generic RecordReader that can hand out different recordReaders
* for each chunk in the CombineFileSplit.
*/
- public CombineFileRecordReader(JobConf job, CombineFileSplit split,
- Reporter reporter,
- Class<RecordReader<K, V>> rrClass)
- throws IOException {
+ public static <K,V> CombineFileRecordReader<K,V> createInstance(JobConf job, CombineFileSplit split,
+ Reporter reporter,
+ Class<RecordReader<K, V>> rrClass) throws IOException{
Review Comment:
```suggestion
Class<RecordReader<K, V>> rrClass) throws IOException {
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
Review Comment:
Thanks. In my current impression, it might be safer to keep using the original status with an annotation. That's because `FileStatus` is heavily extended by various classes, and it is not self-evident that we can clone it as the parent class.
https://github.com/apache/hadoop/blob/rel/release-3.4.1/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/HdfsNamedFileStatus.java
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -51,19 +48,36 @@ public class WebHCatJTShim23 implements WebHCatJTShim {
/**
* Create a connection to the Job Tracker.
*/
- public WebHCatJTShim23(final Configuration conf, final UserGroupInformation ugi)
- throws IOException {
- try {
- this.conf = conf;
- jc = ugi.doAs(new PrivilegedExceptionAction<JobClient>() {
- public JobClient run() throws IOException, InterruptedException {
- //create this in doAs() so that it gets a security context based passed in 'ugi'
- return new JobClient(conf);
- }
- });
+ public static WebHCatJTShim createInstance(final Configuration conf, final UserGroupInformation ugi)
+ throws IOException {
+ int[] flag = new int[1];
+ ArrayList<String> eMessage = new ArrayList<>();
+ WebHCatJTShim webHCatJTShim = new WebHCatJTShim23(conf, ugi, flag, eMessage);
+ if (flag[0] == -1) {
+ throw new IOException("Failed to create job Client" + eMessage.get(0));
+ } else if (flag[0] == -2) {
+ throw new RuntimeException("Failed to create job Client" + eMessage.get(0));
}
- catch(InterruptedException ex) {
- throw new RuntimeException("Failed to create JobClient", ex);
+ return webHCatJTShim;
+ }
+
+ private WebHCatJTShim23(final Configuration conf, final UserGroupInformation ugi, int[] flag,
+ ArrayList<String> eMessage) {
+ this.conf = new Configuration(conf);
+ try {
+ this.jc = ugi.doAs(new PrivilegedExceptionAction<JobClient>() {
+ public JobClient run() throws IOException, InterruptedException {
+ //create this in doAs() so that it gets a security context based passed in 'ugi'
+ return new JobClient(conf);
+ }
+ });
+ } catch (InterruptedException ex) {
+ eMessage.add(ex.getMessage());
+ flag[0] = -2;
+ this.jc = null;
+ } catch (IOException ex) {
+ flag[0] = -1;
+ this.jc = null;
}
Review Comment:
Can we pull out the error-handling from the constructor like this one?
```
public static WebHCatJTShim createInstance(final Configuration conf, final UserGroupInformation ugi)
throws IOException {
JobClient jc;
try {
jc = ugi.doAs((PrivilegedExceptionAction<JobClient>) () -> {
//create this in doAs() so that it gets a security context based passed in 'ugi'
return new JobClient(conf);
});
} catch (InterruptedException e) {
throw new RuntimeException("Failed to create JobClient", e);
}
return new WebHCatJTShim23(conf, jc);
}
private WebHCatJTShim23(final Configuration conf, final JobClient jc) {
this.conf = new Configuration(conf);
this.jc = jc;
}
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052217942
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -37,10 +37,7 @@
import java.io.IOException;
import java.net.URI;
import java.security.PrivilegedExceptionAction;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
Review Comment:
changed to individual imports but `mvn checkstyle:checkstyle -DskipTests -pl shims/0.23` was working fine with * also
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052221487
##########
shims/common/pom.xml:
##########
@@ -106,6 +106,10 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
+ <artifactId>hive-storage-api</artifactId>
+ </dependency>
Review Comment:
[shims/common/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java](https://github.com/apache/hive/pull/5776/files#diff-0f95d1dce726d2eec066cf58c509df20f4787698ff8360715265492e97bd4a6e)
added this in place of dependency
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2816757018
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [8 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051458866
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
Review Comment:
basically this is one of the parameter i am going to pass in the parameterised constructor of fileStatus to prevent `"EI_EXPOSE_REP"` and symlink can throw exception so try catch is used below
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2816722495
> I'm still reviewing this PR. I found that SonarCloud reported some issues. Could you please resolve ones that this PR should address? https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true
and regarding the reliability ones the first one is flagging for re-interrupt for an interruption exception which is handled in factory method of that constructor but sonarcloud is not smart enough to understand that and the other one which is for not closing a closable object inside a createInstance factory method which should not be done because that method is used in place of constructor to create object so it does not make sense to close object inside that method
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051962233
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -51,19 +48,36 @@ public class WebHCatJTShim23 implements WebHCatJTShim {
/**
* Create a connection to the Job Tracker.
*/
- public WebHCatJTShim23(final Configuration conf, final UserGroupInformation ugi)
- throws IOException {
- try {
- this.conf = conf;
- jc = ugi.doAs(new PrivilegedExceptionAction<JobClient>() {
- public JobClient run() throws IOException, InterruptedException {
- //create this in doAs() so that it gets a security context based passed in 'ugi'
- return new JobClient(conf);
- }
- });
+ public static WebHCatJTShim createInstance(final Configuration conf, final UserGroupInformation ugi)
+ throws IOException {
+ int[] flag = new int[1];
+ ArrayList<String> eMessage = new ArrayList<>();
+ WebHCatJTShim webHCatJTShim = new WebHCatJTShim23(conf, ugi, flag, eMessage);
+ if (flag[0] == -1) {
+ throw new IOException("Failed to create job Client" + eMessage.get(0));
+ } else if (flag[0] == -2) {
+ throw new RuntimeException("Failed to create job Client" + eMessage.get(0));
}
- catch(InterruptedException ex) {
- throw new RuntimeException("Failed to create JobClient", ex);
+ return webHCatJTShim;
+ }
+
+ private WebHCatJTShim23(final Configuration conf, final UserGroupInformation ugi, int[] flag,
+ ArrayList<String> eMessage) {
+ this.conf = new Configuration(conf);
+ try {
+ this.jc = ugi.doAs(new PrivilegedExceptionAction<JobClient>() {
+ public JobClient run() throws IOException, InterruptedException {
+ //create this in doAs() so that it gets a security context based passed in 'ugi'
+ return new JobClient(conf);
+ }
+ });
+ } catch (InterruptedException ex) {
+ eMessage.add(ex.getMessage());
+ flag[0] = -2;
+ this.jc = null;
+ } catch (IOException ex) {
+ flag[0] = -1;
+ this.jc = null;
}
Review Comment:
Thanks this one is good approach i added this one instead of the hack i was using π
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051959389
##########
shims/common/src/main/java/org/apache/hadoop/fs/ProxyFileSystem.java:
##########
@@ -207,21 +199,25 @@ public FileStatus[] listStatus(Path f) throws IOException {
@Override //ref. HADOOP-12502
public RemoteIterator<FileStatus> listStatusIterator(Path f) throws IOException {
- return new RemoteIterator<FileStatus>() {
- private final RemoteIterator<FileStatus> orig =
- ProxyFileSystem.super.listStatusIterator(swizzleParamPath(f));
-
- @Override
- public boolean hasNext() throws IOException {
- return orig.hasNext();
- }
-
- @Override
- public FileStatus next() throws IOException {
- FileStatus ret = orig.next();
- return swizzleFileStatus(ret, false);
- }
- };
+ try {
+ final RemoteIterator<FileStatus> remoteIterator = ProxyFileSystem.super.listStatusIterator(swizzleParamPath(f));
+ return new RemoteIterator<FileStatus>() {
+ private final RemoteIterator<FileStatus> orig = remoteIterator;
+
+ @Override
+ public boolean hasNext() throws IOException {
+ return orig.hasNext();
+ }
+
+ @Override
+ public FileStatus next() throws IOException {
+ FileStatus ret = orig.next();
+ return swizzleFileStatus(ret, false);
+ }
+ };
+ } catch (IOException e) {
Review Comment:
yeah, done removed
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2818089945
shims/common/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java
added this in place of dependency
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051441325
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
Review Comment:
```suggestion
symlink = fileStatus.getSymlink();
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
Review Comment:
```suggestion
public static HadoopFileStatus createInstance(Configuration conf, FileSystem fs, Path file) throws IOException {
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
Review Comment:
```suggestion
} catch (IOException e) {
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
Review Comment:
```suggestion
symlink = null;
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
Review Comment:
```suggestion
FileStatus other = fileStatus;
```
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
+ return new FileStatus(other.getLen(), other.isDirectory(), other.getReplication(), other.getBlockSize(), other.getModificationTime(), other.getAccessTime(), other.getPermission(), other.getOwner(), other.getGroup(), symlink, other.getPath());
Review Comment:
The length of a single line has to be smaller than or equal to 120 in our coding style.
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
Review Comment:
I would like to know the intention of this change
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2816616494
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2816719204
> I'm still reviewing this PR. I found that SonarCloud reported some issues. Could you please resolve ones that this PR should address? https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true
regarding the issues flagged by sonarcloud under maintainability issues focussing on exceptions thrown, I haven't made changes to those, they were already present in the codebase i just wrapped them inside a wrapper function in some cases which is to solve spotbugs warnings which is now scanned by sonarcloud and it is flagging them now although I think they are meaningless
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051962350
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1335,23 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin = null;
+ KeyProvider keyP = null;
+ try {
+ hadmin = new HdfsAdmin(uri, conf);
+ keyP = hadmin.getKeyProvider();
+ } catch (IOException e) {
Review Comment:
done removed
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -774,26 +772,28 @@ public FutureDataInputStreamBuilder openFile(Path path) throws IOException, Unsu
}
@Override
- public RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f)
- throws FileNotFoundException, IOException {
- return new RemoteIterator<LocatedFileStatus>() {
- private final RemoteIterator<LocatedFileStatus> stats =
- ProxyFileSystem23.super.listLocatedStatus(
- ProxyFileSystem23.super.swizzleParamPath(f));
-
- @Override
- public boolean hasNext() throws IOException {
- return stats.hasNext();
- }
+ public RemoteIterator<LocatedFileStatus> listLocatedStatus(final Path f) throws FileNotFoundException, IOException {
+ try {
+ final RemoteIterator<LocatedFileStatus> remoteIterator =
+ ProxyFileSystem23.super.listLocatedStatus(ProxyFileSystem23.super.swizzleParamPath(f));
+ return new RemoteIterator<LocatedFileStatus>() {
+ private final RemoteIterator<LocatedFileStatus> stats = remoteIterator;
+
+ @Override
+ public boolean hasNext() throws IOException {
+ return stats.hasNext();
+ }
- @Override
- public LocatedFileStatus next() throws IOException {
- LocatedFileStatus result = stats.next();
- return new LocatedFileStatus(
- ProxyFileSystem23.super.swizzleFileStatus(result, false),
- result.getBlockLocations());
- }
- };
+ @Override
+ public LocatedFileStatus next() throws IOException {
+ LocatedFileStatus result = stats.next();
+ return new LocatedFileStatus(ProxyFileSystem23.super.swizzleFileStatus(result, false),
+ result.getBlockLocations());
+ }
+ };
+ } catch (IOException e) {
Review Comment:
done removed
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5776:
URL: https://github.com/apache/hive/pull/5776#issuecomment-2818204785
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5776&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5776&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5776&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5776)
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052217942
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -37,10 +37,7 @@
import java.io.IOException;
import java.net.URI;
import java.security.PrivilegedExceptionAction;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
Review Comment:
changed to individual imports but `mvn checkstyle:checkstyle -DskipTests -pl shims/0.23` was also working fine with * also
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051458644
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
+ }catch (IOException e){
+ symlink=null;
+ }
+ FileStatus other=fileStatus;
+ return new FileStatus(other.getLen(), other.isDirectory(), other.getReplication(), other.getBlockSize(), other.getModificationTime(), other.getAccessTime(), other.getPermission(), other.getOwner(), other.getGroup(), symlink, other.getPath());
Review Comment:
i used this parameterised constructor to prevent "EI_EXPOSE_REP"
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2051463250
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
+ FileStatus fileStatus = fs.getFileStatus(file);
+ return new HadoopFileStatus(conf, fs, fileStatus, file);
+ }
+
public FileStatus getFileStatus() {
- return fileStatus;
+ Path symlink;
+ try {
+ symlink= fileStatus.getSymlink();
Review Comment:
done
##########
shims/common/src/main/java/org/apache/hadoop/hive/io/HdfsUtils.java:
##########
@@ -211,8 +210,20 @@ public HadoopFileStatus(Configuration conf, FileSystem fs, Path file) throws IOE
this.aclStatus = aclStatus;
}
+ public static HadoopFileStatus createInstance (Configuration conf, FileSystem fs, Path file) throws IOException {
Review Comment:
done
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052163988
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin = null;
+ KeyProvider keyP = null;
Review Comment:
I think we don't need to initialize it as null.
##########
shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java:
##########
@@ -37,10 +37,7 @@
import java.io.IOException;
import java.net.URI;
import java.security.PrivilegedExceptionAction;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
Review Comment:
If I remember correctly, we don't use a wildcard import.
https://github.com/apache/hive/blob/be59bd0b445096e431c18bbfa0d682c78ba18b76/checkstyle/checkstyle.xml#L152-L155
You can run checkstyle with `mvn checkstyle:checkstyle -DskipTests -pl shims/0.23`, and see the result in the target directory. It is unrealistic to fix all issues, but it would be great if we can keep the codestyle consistent as much as possible especially for new changes.
```
cat shims/0.23/target/checkstyle-result.xml | grep -A 1 WebHCatJTShim23
<file name="/Users/shohei.okumiya/ghq/github.com/okumin/hive/shims/0.23/src/main/java/org/apache/hadoop/mapred/WebHCatJTShim23.java">
<error line="40" severity="warning" message="Using the '.*' form of import should be avoided - java.util.*." source="com.puppycrawl.tools.checkstyle.checks.imports.AvoidStarImportCheck"/>
```
##########
shims/common/pom.xml:
##########
@@ -106,6 +106,10 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.hive</groupId>
+ <artifactId>hive-storage-api</artifactId>
+ </dependency>
Review Comment:
That makes sense... Can we move this class to shims-common so that both shims-common and 0.23 can share it? Adding the whole dependency just for the annotation might confuse maintainers or the chaos might cause a future problem.
https://github.com/apache/hive/blob/master/shims/0.23/src/main/java/org/apache/hadoop/util/SuppressFBWarnings.java
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28891: Fix spotbugs issues in hive-shims with spotbugs-maven-plugin 4.8.6.6 [hive]
Posted by "ramitg254 (via GitHub)" <gi...@apache.org>.
ramitg254 commented on code in PR #5776:
URL: https://github.com/apache/hive/pull/5776#discussion_r2052218510
##########
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java:
##########
@@ -1334,10 +1331,19 @@ public static class HdfsEncryptionShim implements HadoopShims.HdfsEncryptionShim
private final Configuration conf;
- public HdfsEncryptionShim(URI uri, Configuration conf) throws IOException {
+ public static HdfsEncryptionShim createInstance(URI uri, Configuration conf) throws IOException {
+ HdfsAdmin hadmin = null;
+ KeyProvider keyP = null;
Review Comment:
removed null
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org