add alertId parameter in get chained alert API and paginate associated alerts if alertId param is mentioned #1071
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d alerts if alertId param is mentioned Signed-off-by: Surya Sashank Nistala <[email protected]>
eirsep
requested review from
lezzago,
AWSHurneyt,
sbcd90,
getsaurabh02,
praveensameneni,
qreshi,
bowenlan-amzn and
rishabhmaurya
as code owners
Codecov Report
@@ Coverage Diff @@
## main #1071 +/- ##
============================================
+ Coverage 67.70% 67.73% +0.03%
Complexity 105 105
============================================
Files 160 160
Lines 10324 10343 +19
Branches 1518 1522 +4
============================================
+ Hits 6990 7006 +16
- Misses 2671 2672 +1
- Partials 663 665 +2
|
lezzago
requested changes
Aug 2, 2023
| actionListener.onResponse(GetWorkflowAlertsResponse(alerts, associatedAlerts, totalAlertCount)) | ||
| } catch (e: Exception) { | ||
| actionListener.onFailure(AlertingException("Failed to get alerts", RestStatus.INTERNAL_SERVER_ERROR, e)) | ||
| } | ||
| } | ||
|
|
||
| private suspend fun getAssociatedAlerts(associatedAlerts: MutableList<Alert>, alerts: MutableList<Alert>, alertIndex: String) { | ||
| private suspend fun getAssociatedAlerts( |
There was a problem hiding this comment.
fixed this and verified in test
| // if alert id is mentioned we cannot set "from" field as it may not return id. we would be using it to paginate associated alerts | ||
| val from = if (getWorkflowAlertsRequest.alertIds.isNullOrEmpty()) | ||
| tableProp.startIndex | ||
| else 0 |
There was a problem hiding this comment.
The startIndex isn't being passed over to getAssociatedAlerts
There was a problem hiding this comment.
we dont need it to be passed over. we will use tableProp.startIndex
we need this check on from param if alertId is mentioned because we cannot paginate for single id in search request to fetch the chained alerts
Signed-off-by: Surya Sashank Nistala <[email protected]>
lezzago
approved these changes
Aug 2, 2023
AWSHurneyt
approved these changes
Aug 2, 2023
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Aug 2, 2023
…d alerts if alertId param is mentioned (#1071) * add alertId parameter in get chained alert API and paginate associated alerts if alertId param is mentioned Signed-off-by: Surya Sashank Nistala <[email protected]> * set from param in associated alerts search request Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> (cherry picked from commit d9f66b3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Aug 2, 2023
…d alerts if alertId param is mentioned (#1071) * add alertId parameter in get chained alert API and paginate associated alerts if alertId param is mentioned Signed-off-by: Surya Sashank Nistala <[email protected]> * set from param in associated alerts search request Signed-off-by: Surya Sashank Nistala <[email protected]> --------- Signed-off-by: Surya Sashank Nistala <[email protected]> (cherry picked from commit d9f66b3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
eirsep
pushed a commit
that referenced
this pull request
Aug 2, 2023
…d alerts if alertId param is mentioned (#1071) (#1074) * add alertId parameter in get chained alert API and paginate associated alerts if alertId param is mentioned * set from param in associated alerts search request --------- (cherry picked from commit d9f66b3) Signed-off-by: Surya Sashank Nistala <[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>
lezzago
pushed a commit
that referenced
this pull request
Aug 23, 2023
…d alerts if alertId param is mentioned (#1071) (#1073) * add alertId parameter in get chained alert API and paginate associated alerts if alertId param is mentioned * set from param in associated alerts search request --------- (cherry picked from commit d9f66b3) Signed-off-by: Surya Sashank Nistala <[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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Get chained alert API will support filtering based on an alertId. The motivation is to fetch associated alerts with support for pagination. The size and sort params would apply to fetch associated alerts in this api when alert id is mentioned
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.