Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add listeners for publisher actions #295

Merged
merged 10 commits into from
Jul 30, 2020
Merged

Add listeners for publisher actions #295

merged 10 commits into from
Jul 30, 2020

Conversation

vigyasharma
Copy link
Contributor

@vigyasharma vigyasharma commented Jul 20, 2020

Issue #: 316
#316

Description of changes: Allows users to plugin different action listeners to publisher. Action suggestions are published to each listener.

Tests: WIP

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -30,10 +33,12 @@

private Collator collator;
private boolean isMuted = false;
private List<ActionListener> actionListeners;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we make this list thread safe ? it looks to me that addActionListener() is called from a different thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should only be called during instantiation of the graph, not during regular scheduling.

@vigyasharma vigyasharma marked this pull request as ready for review July 27, 2020 02:14
@vigyasharma vigyasharma requested review from rguo-aws and sidheart and removed request for yojs July 27, 2020 02:14
import java.util.ArrayList;
import java.util.List;

public class PluginControllerConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a singleton pattern here instead of static? Then we can avoid the PowerMock stuff in the test class and have multiple configs in one JVM context

@codecov
Copy link

codecov bot commented Jul 30, 2020

Codecov Report

Merging #295 into master will increase coverage by 0.08%.
The diff coverage is 90.90%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #295      +/-   ##
============================================
+ Coverage     67.23%   67.32%   +0.08%     
- Complexity     2002     2020      +18     
============================================
  Files           291      295       +4     
  Lines         12924    12974      +50     
  Branches       1057     1062       +5     
============================================
+ Hits           8690     8735      +45     
- Misses         3860     3864       +4     
- Partials        374      375       +1     
Impacted Files Coverage Δ Complexity Δ
...cisionmaker/actions/ModifyCacheCapacityAction.java 84.21% <ø> (+2.15%) 16.00 <0.00> (ø)
...cisionmaker/actions/ModifyQueueCapacityAction.java 88.88% <ø> (+2.40%) 15.00 <0.00> (ø)
...ormanceanalyzer/plugins/PublisherEventsLogger.java 60.00% <60.00%> (ø) 3.00 <3.00> (?)
.../performanceanalyzer/plugins/PluginController.java 91.17% <91.17%> (ø) 12.00 <12.00> (?)
...anceanalyzer/decisionmaker/deciders/Publisher.java 67.39% <100.00%> (+3.10%) 12.00 <1.00> (+2.00)
...sticsearch/performanceanalyzer/plugins/Plugin.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...rmanceanalyzer/plugins/PluginControllerConfig.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)
...analyzer/rca/store/ElasticSearchAnalysisGraph.java 100.00% <100.00%> (ø) 7.00 <0.00> (ø)
.../performanceanalyzer/rca/framework/core/Stats.java 97.82% <0.00%> (-2.18%) 22.00% <0.00%> (-1.00%)
...csearch/performanceanalyzer/rca/RcaController.java 80.76% <0.00%> (-0.55%) 38.00% <0.00%> (-1.00%)
... and 4 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 b603245...d207d65. Read the comment docs.

@vigyasharma vigyasharma merged commit 1180718 into master Jul 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add action listeners to decouple action configuration from invocation
3 participants