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

Add a new endpoint to return resource info #2676

Conversation

justinlin-linkedin
Copy link
Collaborator

Summary

Adding a new endpoint and handler to return some resource information.

Test

Will add unit test

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2023

Codecov Report

Attention: 155 lines in your changes are missing coverage. Please review.

Comparison is base (2c3419c) 72.60% compared to head (3d86b80) 72.43%.

Files Patch % Lines
...m/github/ambry/clustermap/HelixClusterManager.java 3.92% 98 Missing ⚠️
.../github/ambry/frontend/GetResourceInfoHandler.java 16.66% 40 Missing ⚠️
...java/com/github/ambry/clustermap/ResourceInfo.java 0.00% 13 Missing ⚠️
...hub/ambry/frontend/FrontendRestRequestService.java 20.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2676      +/-   ##
============================================
- Coverage     72.60%   72.43%   -0.18%     
- Complexity    11567    11572       +5     
============================================
  Files           809      811       +2     
  Lines         65755    65882     +127     
  Branches       8030     8041      +11     
============================================
- Hits          47741    47720      -21     
- Misses        15389    15543     +154     
+ Partials       2625     2619       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justinlin-linkedin justinlin-linkedin changed the title WIP: add a new endpoint to return resource info Add a new endpoint to return resource info Dec 19, 2023
Copy link
Collaborator

@JingQianCloud JingQianCloud left a comment

Choose a reason for hiding this comment

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

LGTM. And thanks for adding another tooling for the helix clustermap.

int numPartitions = getNumberOfPartitionsInResource(resourceName);
int replicationFactor = 3; // by default it is 3;
String numReplicaStr = getResourceConfig(resourceName, dcName).getNumReplica();
if (numReplicaStr != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may hit exception if the string is not a number.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

then we will get an exception and return 503 back to client

Copy link
Contributor

@Arun-LinkedIn Arun-LinkedIn left a comment

Choose a reason for hiding this comment

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

Thank you for adding this endpoint to get resource information. Just had one comment for typo.

@justinlin-linkedin justinlin-linkedin merged commit 18b722b into linkedin:master Dec 20, 2023
5 checks passed
@justinlin-linkedin justinlin-linkedin deleted the justin/fullautoclustermap branch December 20, 2023 21:57
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