-
Notifications
You must be signed in to change notification settings - Fork 79
Add action to create /_alerting/destinations/ #235
Conversation
* Update to OpenDistro version 1.7.0
* Adds support for ES 7.7.0 * Update the Release Notes * Update the requirement of JDK 14 in README * Update the Java version in 'push-notification-jar' workflow
* change default value of the setting 'alert_history_max_age' to 30 days * change default value of the setting 'alert_history_retention_period' to 60 days
- Use Codecov to visualize the code coverage result - Add Coverage Report Upload action in test-workflow - Add several badges in README
) * Issue #232 : Add action to /_alerting/monitor/{id}, /_alerting/monitor/_search
Codecov Report
@@ Coverage Diff @@
## master #235 +/- ##
============================================
+ Coverage 76.30% 76.96% +0.65%
+ Complexity 162 157 -5
============================================
Files 71 75 +4
Lines 2672 2809 +137
Branches 422 430 +8
============================================
+ Hits 2039 2162 +123
- Misses 415 428 +13
- Partials 218 219 +1
Continue to review full report at Codecov.
|
var destinationId: String | ||
var seqNo: Long | ||
var primaryTerm: Long | ||
var refreshPolicy: WriteRequest.RefreshPolicy | ||
var method: RestRequest.Method | ||
var destination: Destination |
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.
Should we be making all the attributes to the request immutable by using val
?
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.
Made val
var id: String | ||
var version: Long | ||
var seqNo: Long | ||
var primaryTerm: Long | ||
var status: RestStatus | ||
var destination: Destination |
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.
Should these be immutable as well since we would not want to modify them after the response is generated.
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.
Made val
} else { | ||
if (!IndexUtils.scheduledJobIndexUpdated) { | ||
IndexUtils.updateIndexMapping(ScheduledJob.SCHEDULED_JOBS_INDEX, ScheduledJob.SCHEDULED_JOB_TYPE, | ||
ScheduledJobIndices.scheduledJobMappings(), clusterService.state(), client.admin().indices(), | ||
object : ActionListener<AcknowledgedResponse> { | ||
override fun onResponse(response: AcknowledgedResponse) { | ||
onUpdateMappingsResponse(response) | ||
} | ||
override fun onFailure(t: Exception) { | ||
actionListener.onFailure(t) | ||
} | ||
}) | ||
} else { | ||
prepareDestinationIndexing() | ||
} |
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.
Can you change the if block to else if
, so we wont have any nested if else statements?
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.
Good catch, changed.
Issue #, if available:
#232
Description of changes:
#232
Failures responses are also standardized.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.