Skip to content

Unify the minion plug-in package regex path#6980

Merged
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:minion_pluggable
May 27, 2021
Merged

Unify the minion plug-in package regex path#6980
Jackie-Jiang merged 1 commit intoapache:masterfrom
Jackie-Jiang:minion_pluggable

Conversation

@Jackie-Jiang
Copy link
Contributor

Description

Unify minion pluggable class (PinotTaskGenerator, PinotTaskExecutorFactory, MinionEventObserverFactory) package regex path to org.apache.pinot.*.plugin.minion.tasks.*.
Modify SimpleMinionClusterIntegrationTest to use the pluggable classes.

Release Notes

Regex path for pluggable MinionEventObserverFactory is changed from org.apache.pinot.*.event.* to org.apache.pinot.*.plugin.minion.tasks.*

@Jackie-Jiang Jackie-Jiang added the backward-incompat Referenced by PRs that introduce or fix backward compat issues label May 26, 2021
@Jackie-Jiang Jackie-Jiang requested a review from jackjlli May 26, 2021 04:48
@Jackie-Jiang
Copy link
Contributor Author

@jackjlli Please review this PR. It fixes the regex path for MinionEventObserverFactory, which is missed in #6618

@codecov-commenter
Copy link

Codecov Report

Merging #6980 (2315ba7) into master (c76ce2a) will decrease coverage by 7.82%.
The diff coverage is 33.33%.

❗ Current head 2315ba7 differs from pull request most recent head 27ef483. Consider uploading reports for the commit 27ef483 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6980      +/-   ##
============================================
- Coverage     73.37%   65.55%   -7.83%     
  Complexity       12       12              
============================================
  Files          1441     1441              
  Lines         71430    71430              
  Branches      10351    10351              
============================================
- Hits          52415    46823    -5592     
- Misses        15490    21220    +5730     
+ Partials       3525     3387     -138     
Flag Coverage Δ
integration ?
unittests 65.55% <33.33%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...x/core/minion/generator/TaskGeneratorRegistry.java 76.00% <ø> (-4.00%) ⬇️
...not/minion/event/EventObserverFactoryRegistry.java 0.00% <0.00%> (-56.53%) ⬇️
...t/minion/executor/TaskExecutorFactoryRegistry.java 12.00% <100.00%> (-76.00%) ⬇️
...a/org/apache/pinot/minion/metrics/MinionMeter.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/BrokerQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/minion/metrics/MinionQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
...pache/pinot/common/utils/grpc/GrpcQueryClient.java 0.00% <0.00%> (-100.00%) ⬇️
...pinot/minion/exception/TaskCancelledException.java 0.00% <0.00%> (-100.00%) ⬇️
...t/core/startree/plan/StarTreeDocIdSetPlanNode.java 0.00% <0.00%> (-100.00%) ⬇️
.../core/startree/plan/StarTreeTransformPlanNode.java 0.00% <0.00%> (-100.00%) ⬇️
... and 338 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c76ce2a...27ef483. Read the comment docs.

Copy link
Member

@jackjlli jackjlli left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@Jackie-Jiang Jackie-Jiang merged commit 0b5dcb7 into apache:master May 27, 2021
@Jackie-Jiang Jackie-Jiang deleted the minion_pluggable branch May 27, 2021 04:53
@Jackie-Jiang Jackie-Jiang added the release-notes Referenced by PRs that need attention when compiling the next release notes label May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Referenced by PRs that introduce or fix backward compat issues release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants