Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HIVE-27871: Fix formatting problems is YarnQueueHelper #4874

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

maheshrajus
Copy link
Contributor

What changes were proposed in this pull request?

Fix formatting problems is YarnQueueHelper

Why are the changes needed?

For better code readability

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

Fix formatting only. no functionality involved

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

There is a comment here:
https://issues.apache.org/jira/browse/HIVE-27871?focusedCommentId=17785948&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17785948

It is not addressed here, we should handle all the logger lines in this file.

On a side note: I am not sure about this coding practice, why someone needs to define these constants, could have directly used the already defined ones RM_WEBAPP_ADDRESS and so, anyway....

@abstractdog
Copy link
Contributor

There is a comment here: https://issues.apache.org/jira/browse/HIVE-27871?focusedCommentId=17785948&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17785948

It is not addressed here, we should handle all the logger lines in this file.

On a side note: I am not sure about this coding practice, why someone needs to define these constants, could have directly used the already defined ones RM_WEBAPP_ADDRESS and so, anyway....

I agree with @ayushtkn, these static constants make sense if you use it 10-20 times in a class, but that's not the case here, I would simply purge them them and use directly YarnConfiguration

Copy link

sonarcloud bot commented Nov 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

warning The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Copy link
Contributor

@abstractdog abstractdog left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

@abstractdog abstractdog merged commit 44ceefe into apache:master Nov 21, 2023
5 checks passed
tarak271 pushed a commit to tarak271/hive-1 that referenced this pull request Dec 19, 2023
…(Mahesh Raju Somalaraju reviewed by Laszlo Bodor, Ayush Saxena)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants