Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

  • Get rid of unreachable if branches in HddsUtils methods, as reported by Sonar
  • Reduce duplication:
    • getHostNameFromConfigKeys already supports multiple keys for fallback
    • extract the exception about unsupported multi-SCM
  • Use OptionalInt instead of Optional<Integer> for ports
  • Move usage of config key and its default value closer
  • Remove unused getScmServiceRpcAddresses()
  • Flip assertEquals args
  • Avoid String.format in logging
  • Remove @param without description

https://issues.apache.org/jira/browse/HDDS-2505

How was this patch tested?

Unit tests (host/port lookup has good coverage). Also ran ozone acceptance test locally.

@bshashikant
Copy link
Contributor

@adoroszlai , can you please the unit test failures. They seem related.

@adoroszlai
Copy link
Contributor Author

Thanks @bshashikant for checking this. I don't see related unit test failures. But this can definitely wait until @elek manages to get a fully green CI run. Then I can update this PR with those fixes to get more relevant test results.

Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor comment inline.

@elek
Copy link
Member

elek commented Dec 3, 2019

Thanks the patch @adoroszlai and the review @xiaoyuyao I am merging it right now.

The acceptance test failure seems to be unrelated (and will be fixed by #282 )

@elek elek closed this in 5e49d7a Dec 3, 2019
@adoroszlai adoroszlai deleted the HDDS-2505 branch December 3, 2019 10:42
@adoroszlai
Copy link
Contributor Author

Thanks @elek for committing it.

ptlrs pushed a commit to ptlrs/ozone that referenced this pull request Mar 8, 2025
…ID at info level (apache#5481) (apache#183)

(cherry picked from commit 6bf3886)

Co-authored-by: Shivam Kumar <[email protected]>
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.

4 participants