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

Add state interfaces #257

Closed
wants to merge 1 commit into from

Conversation

kaituo
Copy link
Member

@kaituo kaituo commented Oct 14, 2020

Note: since there are a lot of dependencies, I only list the main class and test code to save reviewers' time. The build will fail due to missing dependencies. I will use that PR just for review. will not merge it. Will have a big one in the end and merge once after all review PRs get approved.

Issue #, if available:

Description of changes:

We have different states in AD. For better code readability and reuse, this PR adds three interfaces for our states:

  • Expiring states: Represent a state that can be expired with a duration if not accessed.
  • MaintenanceState: Represent a state that needs to maintain its metadata regularly
  • CleanState: Represent a state organized via detectorId. When deleting a detector's state, we can remove it from the state.

Testing done:

  1. other unit tests cover them
  2. end-to-end testing

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

We have different states in AD.  For better code readability and reuse, this PR adds three interfaces for our states:
* Expiring states: Represent a state that can be expired with a duration if not accessed.
* MaintenanceState: Represent a state that needs to maintain its metadata regularly
* CleanState: Represent a state organized via detectorId.  When deleting a detector's state, we can remove it from the state.

Testing done:
1. other unit tests cover them
2. end-to-end testing
@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #257 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #257   +/-   ##
=========================================
  Coverage     73.01%   73.01%           
  Complexity     1461     1461           
=========================================
  Files           164      164           
  Lines          6834     6834           
  Branches        527      527           
=========================================
  Hits           4990     4990           
  Misses         1594     1594           
  Partials        250      250           
Flag Coverage Δ Complexity Δ
#cli 79.27% <ø> (ø) 0.00 <ø> (ø)

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

@ohltyler
Copy link
Contributor

Could you describe more on where/how these interfaces will be implemented with respect to HC detectors?

@kaituo
Copy link
Member Author

kaituo commented Oct 15, 2020

Could you describe more on where/how these interfaces will be implemented with respect to HC detectors?

You can see examples where it is implemented here: https://github.com/opendistro-for-elasticsearch/anomaly-detection/pull/259/files

kaituo added a commit that referenced this pull request Oct 16, 2020
* Add support filtering the data by one categorical variable

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.
@kaituo kaituo closed this Oct 16, 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.

3 participants