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

Add support filtering the data by one categorical variable #270

Merged
merged 3 commits into from
Oct 16, 2020

Conversation

kaituo
Copy link
Member

@kaituo kaituo commented Oct 16, 2020

Issue #, if available:

Description of changes:

This PR is a conglomerate of the following PRs.

#247
#249
#250
#252
#253
#256
#257
#258
#259
#260
#261
#262
#263
#264
#265
#266
#267
#268
#269

This spreadsheet contains the mappings from files to PR number: https://quip-amazon.com/DiHkAmz9oSLu/HC-PR

Testing done:

  1. Add unit tests except four classes (excluded in build.gradle). Will add them in the later PR.
  2. Manual testing passes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #270 into master will increase coverage by 0.49%.
The diff coverage is 76.26%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #270      +/-   ##
============================================
+ Coverage     72.81%   73.30%   +0.49%     
- Complexity     1464     1752     +288     
============================================
  Files           164      180      +16     
  Lines          6867     8119    +1252     
  Branches        533      670     +137     
============================================
+ Hits           5000     5952     +952     
- Misses         1615     1859     +244     
- Partials        252      308      +56     
Flag Coverage Δ Complexity Δ
#plugin 72.72% <76.26%> (+0.67%) 1752.00 <332.00> (+288.00)

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

Impacted Files Coverage Δ Complexity Δ
...oforelasticsearch/ad/AnomalyDetectorJobRunner.java 75.78% <0.00%> (-0.81%) 35.00 <0.00> (ø)
...elasticsearch/ad/constant/CommonErrorMessages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...ticsearch/ad/constant/CommonMessageAttributes.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...distroforelasticsearch/ad/constant/CommonName.java 66.66% <ø> (ø) 1.00 <0.00> (ø)
...stroforelasticsearch/ad/model/AnomalyDetector.java 65.17% <ø> (+3.10%) 55.00 <0.00> (+3.00)
...opendistroforelasticsearch/ad/util/IndexUtils.java 87.87% <ø> (ø) 10.00 <0.00> (ø)
.../opendistroforelasticsearch/ad/util/Throttler.java 100.00% <ø> (ø) 5.00 <0.00> (ø)
...ad/transport/handler/MultiEntityResultHandler.java 17.50% <17.50%> (ø) 2.00 <2.00> (?)
...arch/ad/transport/handler/AnomalyIndexHandler.java 85.71% <25.00%> (-2.82%) 16.00 <0.00> (ø)
...rch/ad/transport/AnomalyResultTransportAction.java 67.18% <27.95%> (-11.41%) 56.00 <8.00> (-3.00)
... and 57 more

@yizheliu-amazon
Copy link
Contributor

https://github.com/opendistro-for-elasticsearch/anomaly-detection/pull/270/checks?check_run_id=1266108900

Rule violated for class com.amazon.opendistroforelasticsearch.ad.caching.PriorityCache.1: lines covered ratio is 0.60, but expected minimum is 0.75

You may also increase coverage for it later to unblock build.

Copy link
Contributor

@yizheliu-amazon yizheliu-amazon left a comment

Choose a reason for hiding this comment

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

approving. please make sure build succeeds afterwards.

@kaituo
Copy link
Member Author

kaituo commented Oct 16, 2020

approving. please make sure build succeeds afterwards.

Sure, thanks YIzhe.

Copy link
Contributor

@ylwu-amzn ylwu-amzn 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 for the change

@kaituo kaituo merged commit b1ec6b8 into opendistro-for-elasticsearch:master Oct 16, 2020
@ohltyler ohltyler added the feature new feature label Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants