You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "KiranVelumuri (via GitHub)" <gi...@apache.org> on 2025/09/17 13:20:40 UTC
[PR] HIVE-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
KiranVelumuri opened a new pull request, #6081:
URL: https://github.com/apache/hive/pull/6081
<!--
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.
-->
Warn instead of throw error in logs when KafkaDagCredentialSupplier is not available in classpath
### 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.
-->
Stacktrace for ClassNotFoundException for KafkaDagCredentialSupplier is printed for cases not relevant to current scenario(might not be Kafka related or need a credential supplier)
### 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.
-->
mvn test -Dtest=TestBeeLineWithArgs#testRowsAffected
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3382097342
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081)
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416170884
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -314,7 +350,46 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
LOG.error("Failed to add credential supplier", e);
}
}
- return dagSuppliers;
+ credentialSuppliers.addAll(dagSuppliers);
+ }
+
+ /**
+ * Returns the security protocol if one is defined in the properties and null otherwise.
+ * <p>The following properties are examined to determine the protocol:</p>
+ * <ol>
+ * <li>security.protocol</li>
+ * <li>kafka.consumer.security.protocol</li>
+ * <li>kafka.producer.security.protocol</li>
+ * </ol>
+ * <p>and the first non null/not empty is returned.</p>
+ * <p>Defining multiple security protocols at the same time is invalid but this method is lenient and tries to pick
+ * the most reasonable option.</p>
+ * @param props the properties from which to obtain the protocol.
+ * @return the security protocol if one is defined in the properties and null otherwise.
+ */
+ static SecurityProtocol getSecurityProtocol(Properties props) {
Review Comment:
i think this functionality should be extracted to util class like `KafkaUtils`
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416141229
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -286,6 +306,23 @@ private void getCredentialsFromSuppliers(BaseWork work, Set<TableDesc> tables, D
if (!UserGroupInformation.isSecurityEnabled()){
return;
}
+ Map<String, PartitionDesc> partitions = ((MapWork) work).getAliasToPartnInfo();
+
+ // We don't need to iterate on all partitions, and check the same TableDesc.
+ PartitionDesc partition = partitions.values().stream().findFirst().orElse(null);
Review Comment:
could there be a partition outside of a tables set?
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -314,7 +350,46 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
LOG.error("Failed to add credential supplier", e);
}
}
- return dagSuppliers;
+ credentialSuppliers.addAll(dagSuppliers);
+ }
+
+ /**
+ * Returns the security protocol if one is defined in the properties and null otherwise.
+ * <p>The following properties are examined to determine the protocol:</p>
+ * <ol>
+ * <li>security.protocol</li>
+ * <li>kafka.consumer.security.protocol</li>
+ * <li>kafka.producer.security.protocol</li>
+ * </ol>
+ * <p>and the first non null/not empty is returned.</p>
+ * <p>Defining multiple security protocols at the same time is invalid but this method is lenient and tries to pick
+ * the most reasonable option.</p>
+ * @param props the properties from which to obtain the protocol.
+ * @return the security protocol if one is defined in the properties and null otherwise.
+ */
+ static SecurityProtocol getSecurityProtocol(Properties props) {
+ String[] securityProtocolConfigs = new String[] { CommonClientConfigs.SECURITY_PROTOCOL_CONFIG,
+ CONSUMER_CONFIGURATION_PREFIX + "." + CommonClientConfigs.SECURITY_PROTOCOL_CONFIG,
+ PRODUCER_CONFIGURATION_PREFIX + "." + CommonClientConfigs.SECURITY_PROTOCOL_CONFIG };
+ for (String c : securityProtocolConfigs) {
+ String v = props.getProperty(c);
+ if (v != null && !v.isEmpty()) {
+ return SecurityProtocol.forName(v);
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns whether a Kafka token is required for performing operations on the specified table.
+ * If "security.protocol" is set to "PLAINTEXT", we don't need to collect delegation token at all.
+ *
+ * @return true if a Kafka token is required for performing operations on the specified table and false otherwise.
+ */
+ private boolean isTokenRequired(TableDesc tableDesc) {
Review Comment:
could we make it more descriptive, like isKafkaTokenRequired?
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416141229
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -286,6 +306,23 @@ private void getCredentialsFromSuppliers(BaseWork work, Set<TableDesc> tables, D
if (!UserGroupInformation.isSecurityEnabled()){
return;
}
+ Map<String, PartitionDesc> partitions = ((MapWork) work).getAliasToPartnInfo();
+
+ // We don't need to iterate on all partitions, and check the same TableDesc.
+ PartitionDesc partition = partitions.values().stream().findFirst().orElse(null);
Review Comment:
could there be a partition outside of a tables set?
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #6081:
URL: https://github.com/apache/hive/pull/6081
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3302999994
# @check-spelling-bot Report
### :red_circle: Please review
See the [files](6081/files/) view or the [action log](https://github.com/apache/hive/actions/runs/17799020584) for details.
#### Unrecognized words (6)
calcualtion
Chrono
getenv
ntz
OOM
unsign
<details><summary>Previously acknowledged words that are now absent
</summary>www </details>
<details><summary>To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words),
run the following commands</summary>
... in a clone of the [git@github.com:KiranVelumuri/hive.git](https://github.com/KiranVelumuri/hive.git) repository
on the `HIVE-28965` branch:
```
update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}
comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/apache/hive/issues/comments/3302999994" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json
patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")
update_files
rm $comment_body
git add -u
```
</details>
<!--
{% comment %}
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.
{% endcomment %}
-->
<!-- See https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples%3A-advice --> <!-- markdownlint-disable MD033 MD041 -->
<details><summary>If the flagged items do not appear to be text</summary>
If items relate to a ...
* well-formed pattern.
If you can write a [pattern](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples:-patterns) that would match it,
try adding it to the `patterns.txt` file.
Patterns are Perl 5 Regular Expressions - you can [test](
https://www.regexplanet.com/advanced/perl/) yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
* binary file.
Please add a file path to the `excludes.txt` file matching the containing file.
File paths are Perl 5 Regular Expressions - you can [test](
https://www.regexplanet.com/advanced/perl/) yours before committing to verify it will match your files.
`^` refers to the file's path from the root of the repository, so `^README\.md$` would exclude [README.md](
../tree/HEAD/README.md) (on whichever branch you're using).
</details>
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "KiranVelumuri (via GitHub)" <gi...@apache.org>.
KiranVelumuri commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2359598204
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -313,7 +313,7 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
Class<? extends DagCredentialSupplier> c = Class.forName(s).asSubclass(DagCredentialSupplier.class);
dagSuppliers.add(c.getConstructor().newInstance());
} catch (ReflectiveOperationException e) {
- LOG.error("Failed to add credential supplier", e);
+ LOG.warn("Failed to add credential supplier: {}", s);
Review Comment:
sure, would look into this.
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416145503
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -314,7 +350,46 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
LOG.error("Failed to add credential supplier", e);
}
}
- return dagSuppliers;
+ credentialSuppliers.addAll(dagSuppliers);
+ }
+
+ /**
+ * Returns the security protocol if one is defined in the properties and null otherwise.
+ * <p>The following properties are examined to determine the protocol:</p>
+ * <ol>
+ * <li>security.protocol</li>
+ * <li>kafka.consumer.security.protocol</li>
+ * <li>kafka.producer.security.protocol</li>
+ * </ol>
+ * <p>and the first non null/not empty is returned.</p>
+ * <p>Defining multiple security protocols at the same time is invalid but this method is lenient and tries to pick
+ * the most reasonable option.</p>
+ * @param props the properties from which to obtain the protocol.
+ * @return the security protocol if one is defined in the properties and null otherwise.
+ */
+ static SecurityProtocol getSecurityProtocol(Properties props) {
+ String[] securityProtocolConfigs = new String[] { CommonClientConfigs.SECURITY_PROTOCOL_CONFIG,
+ CONSUMER_CONFIGURATION_PREFIX + "." + CommonClientConfigs.SECURITY_PROTOCOL_CONFIG,
+ PRODUCER_CONFIGURATION_PREFIX + "." + CommonClientConfigs.SECURITY_PROTOCOL_CONFIG };
+ for (String c : securityProtocolConfigs) {
+ String v = props.getProperty(c);
+ if (v != null && !v.isEmpty()) {
+ return SecurityProtocol.forName(v);
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns whether a Kafka token is required for performing operations on the specified table.
+ * If "security.protocol" is set to "PLAINTEXT", we don't need to collect delegation token at all.
+ *
+ * @return true if a Kafka token is required for performing operations on the specified table and false otherwise.
+ */
+ private boolean isTokenRequired(TableDesc tableDesc) {
Review Comment:
could we make it more descriptive, like isKafkaTokenRequired?
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3385121753
i've updated the snippet, @KiranVelumuri please take a look https://github.com/apache/hive/pull/6081#issuecomment-3385045320
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3303336808
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081)
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416154556
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -179,13 +183,29 @@ public class DagUtils {
public static final String TEZ_TMP_DIR_KEY = "_hive_tez_tmp_dir";
private static final Logger LOG = LoggerFactory.getLogger(DagUtils.class.getName());
private static final String TEZ_DIR = "_tez_scratch_dir";
- private static final DagUtils instance = new DagUtils(defaultCredentialSuppliers());
+ private static final DagUtils instance = new DagUtils();
// The merge file being currently processed.
public static final String TEZ_MERGE_CURRENT_MERGE_FILE_PREFIX =
"hive.tez.current.merge.file.prefix";
// A comma separated list of work names used as prefix.
public static final String TEZ_MERGE_WORK_FILE_PREFIXES = "hive.tez.merge.file.prefixes";
- private final List<DagCredentialSupplier> credentialSuppliers;
+ private static final List<DagCredentialSupplier> credentialSuppliers = new ArrayList<>();
+
+ /**
+ * MANDATORY Table property indicating kafka broker(s) connection string.
+ */
+ private static final String HIVE_KAFKA_BOOTSTRAP_SERVERS = "kafka.bootstrap.servers";
Review Comment:
could we use constants from KafkaTableProperties?
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416154556
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -179,13 +183,29 @@ public class DagUtils {
public static final String TEZ_TMP_DIR_KEY = "_hive_tez_tmp_dir";
private static final Logger LOG = LoggerFactory.getLogger(DagUtils.class.getName());
private static final String TEZ_DIR = "_tez_scratch_dir";
- private static final DagUtils instance = new DagUtils(defaultCredentialSuppliers());
+ private static final DagUtils instance = new DagUtils();
// The merge file being currently processed.
public static final String TEZ_MERGE_CURRENT_MERGE_FILE_PREFIX =
"hive.tez.current.merge.file.prefix";
// A comma separated list of work names used as prefix.
public static final String TEZ_MERGE_WORK_FILE_PREFIXES = "hive.tez.merge.file.prefixes";
- private final List<DagCredentialSupplier> credentialSuppliers;
+ private static final List<DagCredentialSupplier> credentialSuppliers = new ArrayList<>();
+
+ /**
+ * MANDATORY Table property indicating kafka broker(s) connection string.
+ */
+ private static final String HIVE_KAFKA_BOOTSTRAP_SERVERS = "kafka.bootstrap.servers";
Review Comment:
could we use constants from KafkaTableProperties?
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3385045320
i my opinion it's a bit overkill. We shouldn't copy code from kafka-handler. we should initialize suppliers lazily only when UserGroupInformation.isSecurityEnabled
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "KiranVelumuri (via GitHub)" <gi...@apache.org>.
KiranVelumuri commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3385101890
yes @deniskuzZ. This is what I felt too and wanted your opinion on.
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2355605016
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -313,7 +313,7 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
Class<? extends DagCredentialSupplier> c = Class.forName(s).asSubclass(DagCredentialSupplier.class);
dagSuppliers.add(c.getConstructor().newInstance());
} catch (ReflectiveOperationException e) {
- LOG.error("Failed to add credential supplier", e);
+ LOG.warn("Failed to add credential supplier: {}", s);
Review Comment:
that's ok, but would be great to init those CredentialSuppliers only when used
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "KiranVelumuri (via GitHub)" <gi...@apache.org>.
KiranVelumuri commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3395776791
> i've updated the snippet, @KiranVelumuri please take a look [#6081 (comment)](https://github.com/apache/hive/pull/6081#issuecomment-3385045320)
@deniskuzZ Thank you for the snippet. Could you please review? Sorry I was away travelling for the past 3 days.
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "KiranVelumuri (via GitHub)" <gi...@apache.org>.
KiranVelumuri commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3385007819
@deniskuzZ We needed some check at DagUtils to add the kafka credential supplier only when needed. So I referred to obtainToken and added those checks(isTokenRequired) here.
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "KiranVelumuri (via GitHub)" <gi...@apache.org>.
KiranVelumuri commented on code in PR #6081:
URL: https://github.com/apache/hive/pull/6081#discussion_r2416181883
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java:
##########
@@ -314,7 +350,46 @@ private static List<DagCredentialSupplier> defaultCredentialSuppliers() {
LOG.error("Failed to add credential supplier", e);
}
}
- return dagSuppliers;
+ credentialSuppliers.addAll(dagSuppliers);
+ }
+
+ /**
+ * Returns the security protocol if one is defined in the properties and null otherwise.
+ * <p>The following properties are examined to determine the protocol:</p>
+ * <ol>
+ * <li>security.protocol</li>
+ * <li>kafka.consumer.security.protocol</li>
+ * <li>kafka.producer.security.protocol</li>
+ * </ol>
+ * <p>and the first non null/not empty is returned.</p>
+ * <p>Defining multiple security protocols at the same time is invalid but this method is lenient and tries to pick
+ * the most reasonable option.</p>
+ * @param props the properties from which to obtain the protocol.
+ * @return the security protocol if one is defined in the properties and null otherwise.
+ */
+ static SecurityProtocol getSecurityProtocol(Properties props) {
Review Comment:
This was from KafkaUtils, I put it here to avoid dependency on kafka-handler(since it causes 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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3385102347
> in my opinion it's a bit of overkill. We shouldn't copy code from kafka-handler, instead we might initialize suppliers lazily only when UserGroupInformation.isSecurityEnabled cc @abstractdog
>
> ```
> private void getCredentialsFromSuppliers(BaseWork work, Set<TableDesc> tables, DAG dag, JobConf conf) {
> if (!UserGroupInformation.isSecurityEnabled()){
> return;
> }
> ..
> lazyInitCredentialSuppliers();
> }
> ```
>
> Note, this is only a test classpath issue, in production deployment kafka-handler jar would be always present.
the original goal was to completely remove kafka dependency from ql, which is the case for every other third party that hive provides a storage handler for, so we should maintain that behavior
I agree with trying the lazy approach if security is disabled as it would easily solve the test log noise
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3384392410
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081) **Quality Gate passed**
Issues
 [7 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081)
--
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-28965: tez.DagUtils: Failed to add credential supplier, ClassNotFoundException: org.apache.hadoop.hive.kafka.KafkaDagCredentialSupplier [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #6081:
URL: https://github.com/apache/hive/pull/6081#issuecomment-3394900264
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=6081&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=6081&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=6081&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=6081)
--
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