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

Allow PintoHelixResourceManager subclasses to be used in the controller starter by providing an overridable PinotHelixResouceManager object creator function #13495

Conversation

9aman
Copy link
Contributor

@9aman 9aman commented Jun 27, 2024

Current Scenario

In BaseControllerStarter, the PinotHelixResourceManager _helixResourceManager initialization is hard coded to the PinotHelixResourceManager object.

This reduces the flexibility of using subclasses of PinotHelixResourceManager in any subclass of the BaseControllerStarter.

Changes in the PR

Introduce an overridable function for initialization of PinotHelixResourceManager _helixResourceManager. Thus, any subclass of PinotHelixResourceManager can be used for initialization in the subclass of BaseControllerStarter.

…er starter

by providing an overridable PinotHelixResouceManager object creator function
@9aman 9aman force-pushed the include_subclassing_capabilites_for_pinot_helix_resource_manager branch 2 times, most recently from 4ae2dca to 165d96f Compare June 27, 2024 07:48
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 62.02%. Comparing base (59551e4) to head (c04ed24).
Report is 690 commits behind head on master.

Files Patch % Lines
...ntroller/helix/core/PinotHelixResourceManager.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #13495      +/-   ##
============================================
+ Coverage     61.75%   62.02%   +0.27%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2559     +123     
  Lines        133233   141362    +8129     
  Branches      20636    21928    +1292     
============================================
+ Hits          82274    87682    +5408     
- Misses        44911    47021    +2110     
- Partials       6048     6659     +611     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.97% <66.66%> (+0.26%) ⬆️
java-21 61.91% <66.66%> (+0.29%) ⬆️
skip-bytebuffers-false 62.00% <66.66%> (+0.25%) ⬆️
skip-bytebuffers-true 61.89% <66.66%> (+34.16%) ⬆️
temurin 62.02% <66.66%> (+0.27%) ⬆️
unittests 62.02% <66.66%> (+0.27%) ⬆️
unittests1 46.66% <ø> (-0.24%) ⬇️
unittests2 27.55% <66.66%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

…propertyStore to allow access from subclasses.
@9aman 9aman force-pushed the include_subclassing_capabilites_for_pinot_helix_resource_manager branch from 165d96f to 5cf1066 Compare June 27, 2024 11:19
…eControllerStarter.java

Co-authored-by: Xiaotian (Jackie) Jiang <[email protected]>
@9aman 9aman force-pushed the include_subclassing_capabilites_for_pinot_helix_resource_manager branch from 145b80a to 7bdcf16 Compare June 28, 2024 03:56
@9aman 9aman force-pushed the include_subclassing_capabilites_for_pinot_helix_resource_manager branch from 7bdcf16 to c04ed24 Compare June 28, 2024 06:47
@klsince klsince merged commit d7fff77 into apache:master Jun 28, 2024
20 checks passed
suyashpatel98 pushed a commit to suyashpatel98/pinot that referenced this pull request Jul 6, 2024
…er starter by providing an overridable PinotHelixResouceManager object creator function (apache#13495)

* Allow PintoHelixResourceManager subclasses to be used in the controller starter
by providing an overridable PinotHelixResouceManager object creator function

---------

Co-authored-by: Aman Khanchandani <[email protected]>
Co-authored-by: Xiaotian (Jackie) Jiang <[email protected]>
yashmayya pushed a commit to yashmayya/pinot that referenced this pull request Jul 24, 2024
…er starter by providing an overridable PinotHelixResouceManager object creator function (apache#13495)

* Allow PintoHelixResourceManager subclasses to be used in the controller starter
by providing an overridable PinotHelixResouceManager object creator function

---------

Co-authored-by: Aman Khanchandani <[email protected]>
Co-authored-by: Xiaotian (Jackie) Jiang <[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