Ppl main #1960
Ppl main #1960
Conversation
| manual: Boolean, // if execute was called by user or by scheduled job | ||
| monitorV2Id: String?, | ||
| monitorV2: MonitorV2?, | ||
| requestStart: TimeValue? = null, |
There was a problem hiding this comment.
I went through the code, this is not actually used. It is only used once to compare this with requestEnd and if they are the same, it throws an exception. Based on that, we should just remove requestStart. This was needed in v1 monitors because query level and bucket level monitors lets you run the monitor execution within a specified time window. That does not happen for ppl monitors.
| monitorV2Id: String?, | ||
| monitorV2Ids: List<String>? = null, |
There was a problem hiding this comment.
[Blocking since decides the APIs and we can just go back on it] Why not just use monitorV2Ids here? If it is just one id, it can still be put into monitorV2Ids
| val scheduledJob = ScheduledJob.parse(xcp, getResponse.id, getResponse.version) | ||
|
|
||
| validateMonitorV1(scheduledJob)?.let { | ||
| actionListener.onFailure(AlertingException.wrap(it)) |
There was a problem hiding this comment.
We should throw an exception that says trying to delete a v2 monitor using the v1 monitor and to use the v2 api.
There was a problem hiding this comment.
We should do the same for the other v1 apis
There was a problem hiding this comment.
The validateMonitorV1 and validateMonitorV2 functions returns exactly such an error, and calls onFailure with that error (wrapped in AlertingException)
| ) | ||
| return ScheduledJob.parse(xcp, getResponse.id, getResponse.version) as Workflow | ||
| val scheduledJob = ScheduledJob.parse(xcp, getResponse.id, getResponse.version) | ||
| validateMonitorV1(scheduledJob)?.let { |
There was a problem hiding this comment.
Should we update the function name to include workflows to make it clear?
| } | ||
|
|
||
| // Checks if the exception is caused by an IndexNotFoundException (directly or nested). | ||
| private fun isIndexNotFoundException(e: Exception): Boolean { |
There was a problem hiding this comment.
It is moved to AlertingV2Utils. The PR to add (copy) it to AlertingV2Utils is merged, so the diff shows the deletion of it from where the function originally came from.
| fun getEmptySearchResponse(): SearchResponse { | ||
| val internalSearchResponse = InternalSearchResponse( | ||
| SearchHits(emptyArray(), TotalHits(0L, Relation.EQUAL_TO), 0.0f), | ||
| InternalAggregations.from(Collections.emptyList()), | ||
| Suggest(Collections.emptyList()), | ||
| SearchProfileShardResults(Collections.emptyMap()), | ||
| false, | ||
| false, | ||
| 0 | ||
| ) | ||
|
|
||
| return SearchResponse( | ||
| internalSearchResponse, | ||
| "", | ||
| 0, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| ShardSearchFailure.EMPTY_ARRAY, | ||
| SearchResponse.Clusters.EMPTY | ||
| ) | ||
| } | ||
|
|
||
| // Checks if the exception is caused by an IndexNotFoundException (directly or nested). | ||
| private fun isIndexNotFoundException(e: Exception): Boolean { | ||
| if (e is IndexNotFoundException) return true | ||
| if (e is RemoteTransportException) { | ||
| val cause = e.cause | ||
| if (cause is IndexNotFoundException) return true | ||
| } | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
It is moved to AlertingV2Utils. The PR to add (copy) it to AlertingV2Utils is merged, so the diff shows the deletion of it from where the function originally came from.
| // get execution time interval | ||
| val (periodStart, periodEnd) = if (execMonitorV2Request.requestStart != null) { | ||
| Pair( | ||
| Instant.ofEpochMilli(execMonitorV2Request.requestStart.millis), |
There was a problem hiding this comment.
requestStart is not needed since it isn't really used.
toepkerd
force-pushed
the
ppl-main
branch
2 times, most recently
from
f43af57 to
7538f94
Compare
Signed-off-by: Dennis Toepker <toepkerd@amazon.com>
3 participants
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.