PPL Alerting: Delete Monitor, More V1/V2 Separation #1968
Conversation
Signed-off-by: Dennis Toepker <[email protected]>
toepkerd
requested review from
AWSHurneyt,
amsiglan,
bowenlan-amzn,
eirsep,
engechas,
getsaurabh02,
goyamegh,
jowg-amazon,
lezzago,
praveensameneni,
rishabhmaurya,
riysaxen-amzn and
sbcd90
as code owners
| val scheduledJob = ScheduledJob.parse(xcp, getResponse.id, getResponse.version) | ||
|
|
||
| validateMonitorV1(scheduledJob)?.let { | ||
| actionListener.onFailure(AlertingException.wrap(it)) |
There was a problem hiding this comment.
Improve the exception thrown that the user is using the wrong delete api. Please do the same for the other v1 apis
There was a problem hiding this comment.
Will update the error message given to users to more explicitly state that the wrong API was used and make it more actionable, etc
| } | ||
| } | ||
|
|
||
| fun getEmptySearchResponse(): SearchResponse { |
There was a problem hiding this comment.
The functions are moved from TransportSearchMonitorAction to AlertingV2Utils. This logic isn't actually being deleted. Previous PRs included this util function in AlertingV2Utils, and this later PR catches up to delete it from where it came from.
| } | ||
|
|
||
| // 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.
The functions are moved from TransportSearchMonitorAction to AlertingV2Utils. This logic isn't actually being deleted. Previous PRs included this util function in AlertingV2Utils, and this later PR catches up to delete it from where it came from.
| val scheduledJob = ScheduledJob.parse(xcp, getResponse.id, getResponse.version) | ||
|
|
||
| AlertingV2Utils.validateMonitorV2(scheduledJob)?.let { | ||
| actionListener.onFailure(AlertingException.wrap(it)) |
There was a problem hiding this comment.
we should inform that the user needs to use the v1 api to delete
There was a problem hiding this comment.
Will update the error message given to users to more explicitly state that the wrong API was used and make it more actionable, etc
AWSHurneyt
left a comment
AWSHurneyt
left a comment
There was a problem hiding this comment.
Not blocking:
My only comment is that it seems like separating some of these API into versions for V1 and V2 seems to just make managing the plugin assets via API more complex for customers.
Considering the assets these API interact with (V1 and V2 monitors) are all stored in the same index, and each asset has a unique ID, requiring customers to call different API in order to delete/get the asset via that ID seems overly complex. The V1/V2 split SearchMonitor API could probably be accomplished by using a filter either in the query, or a query param in the URI.
Signed-off-by: Dennis Toepker <[email protected]>
* PPL Alerting: Delete Monitor, More V1/V2 Separation Signed-off-by: Dennis Toepker <[email protected]> * making v1 v2 separation error messaging more actionable Signed-off-by: Dennis Toepker <[email protected]> --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]> (cherry picked from commit fd151de) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* PPL Alerting: Delete Monitor, More V1/V2 Separation * making v1 v2 separation error messaging more actionable --------- (cherry picked from commit fd151de) Signed-off-by: Dennis Toepker <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Dennis Toepker <[email protected]>
…ct#1968) * PPL Alerting: Delete Monitor, More V1/V2 Separation Signed-off-by: Dennis Toepker <[email protected]> * making v1 v2 separation error messaging more actionable Signed-off-by: Dennis Toepker <[email protected]> --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]>
* PPL Alerting: Delete Monitor, More V1/V2 Separation * making v1 v2 separation error messaging more actionable --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]>
4 participants
Description
Delete Monitor functionality and V1/V2 API separation measures
Related Issues
#1880
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.