-
Notifications
You must be signed in to change notification settings - Fork 73
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
Not blocking detector creation on unknown feature validation error #1366
Not blocking detector creation on unknown feature validation error #1366
Conversation
Signed-off-by: Amit Galitzky <[email protected]>
src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java
Show resolved
Hide resolved
Signed-off-by: Amit Galitzky <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1366 +/- ##
============================================
+ Coverage 80.02% 80.04% +0.01%
- Complexity 5664 5674 +10
============================================
Files 533 533
Lines 23434 23438 +4
Branches 2334 2334
============================================
+ Hits 18753 18760 +7
- Misses 3572 3578 +6
+ Partials 1109 1100 -9
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…at least not always consistent now Signed-off-by: Amit Galitzky <[email protected]>
f9b833c
to
9a98d18
Compare
...g/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java
Outdated
Show resolved
Hide resolved
...g/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java
Outdated
Show resolved
Hide resolved
...g/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java
Show resolved
Hide resolved
...g/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Amit Galitzky <[email protected]>
18294a4
to
a0aa89f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much cleaner! Thanks for addressing the comments
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/anomaly-detection/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/anomaly-detection/backport-2.x
# Create a new branch
git switch --create backport/backport-1366-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4c545ab27d78be4859afde612719a4367c342f9e
# Push it to GitHub
git push --set-upstream origin backport/backport-1366-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/anomaly-detection/backport-2.x Then, create a pull request where the |
…pensearch-project#1366) * don't fail on unknown exception Signed-off-by: Amit Galitzky <[email protected]> * fixing test Signed-off-by: Amit Galitzky <[email protected]> * order of needed permissions is changed on latest security version or at least not always consistent now Signed-off-by: Amit Galitzky <[email protected]> * refactor customNodeclient Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]>
…pensearch-project#1366) * don't fail on unknown exception Signed-off-by: Amit Galitzky <[email protected]> * fixing test Signed-off-by: Amit Galitzky <[email protected]> * order of needed permissions is changed on latest security version or at least not always consistent now Signed-off-by: Amit Galitzky <[email protected]> * refactor customNodeclient Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]>
…1366) (#1371) * don't fail on unknown exception * fixing test * order of needed permissions is changed on latest security version or at least not always consistent now * refactor customNodeclient --------- Signed-off-by: Amit Galitzky <[email protected]>
Description
Currently during detector creation we check for a few different things in relation to the configured feature. If aggregation is valid, if we have at least one feature, that we don't aggregate over a non numerical field with something like average.
However we currently fail on any seen exception, we want to change this that if there is an unknown exception that we don't completely block detector creation. This is due to the fact that we might get things like timeout or search cancellation that are related to the cluster, not cause the configuration is wrong. Users should be able to still create the detector and handle any cluster or settings changes they need to later on if it effects the actual detector creation.
Related Issues
Resolves #1365
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.