Skip to content

NIFI-13722: Kerberos ticket renewal issue due static thread pool in Iceberg library (nifi-1.x)#9305

Closed
mark-bathori wants to merge 1 commit intoapache:support/nifi-1.xfrom
mark-bathori:NIFI-13722-1.x
Closed

NIFI-13722: Kerberos ticket renewal issue due static thread pool in Iceberg library (nifi-1.x)#9305
mark-bathori wants to merge 1 commit intoapache:support/nifi-1.xfrom
mark-bathori:NIFI-13722-1.x

Conversation

@mark-bathori
Copy link
Contributor

Summary

NIFI-13722

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 21

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

Comment on lines +86 to +91
KerberosUser kerberosUser;
if (kerberosUserReference.get() == null) {
kerberosUser = kerberosUserService.createKerberosUser();
} else {
kerberosUser = kerberosUserReference.get();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just call get() once

Suggested change
KerberosUser kerberosUser;
if (kerberosUserReference.get() == null) {
kerberosUser = kerberosUserService.createKerberosUser();
} else {
kerberosUser = kerberosUserReference.get();
}
KerberosUser kerberosUser = kerberosUserReference.get() ;
if (kerberosUser == null) {
kerberosUser = kerberosUserService.createKerberosUser();
}

getLogger().error("Privileged action failed with kerberos user " + kerberosUser, e);
session.transfer(session.penalize(flowFile), REL_FAILURE);
if (!handleAuthErrors(e, session, context)) {
getLogger().error("Privileged action failed with kerberos user " + kerberosUser, e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use interpolation and not concatenation.

Suggested change
getLogger().error("Privileged action failed with kerberos user " + kerberosUser, e);
getLogger().error("Privileged action failed with kerberos user {}" , kerberosUser, e);

@mark-bathori
Copy link
Contributor Author

mark-bathori commented Sep 24, 2024

Thanks @dan-s1 for the comments. This is a backport PR so these changes are already merged into the main branch. To keep the consistency between the two branch I would keep the changes as it is, but your requested changes could be done in a separate PR if necessary.

Copy link
Contributor

@turcsanyip turcsanyip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per @mark-bathori comment, it is a backport PR and I would also keep it as it is.

Thanks @dan-s1 for your review comments, I overlooked them in the original review on the main branch. We can address them in a follow-up PR or next time when we touch the Iceberg processors.

+1 Merging this for now.

turcsanyip pushed a commit that referenced this pull request Sep 24, 2024
…ceberg library

This closes #9305.

Signed-off-by: Peter Turcsanyi <turcsanyi@apache.org>
@turcsanyip
Copy link
Contributor

Merged.

@turcsanyip turcsanyip closed this Sep 24, 2024
takraj pushed a commit to takraj/nifi that referenced this pull request Mar 17, 2025
…ceberg library

This closes apache#9305.

Signed-off-by: Peter Turcsanyi <turcsanyi@apache.org>
(cherry picked from commit a0ec730)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants