-
Notifications
You must be signed in to change notification settings - Fork 30
WIP: Expose mediator-scan API through the Scan app #2039
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
base: main
Are you sure you want to change the base?
WIP: Expose mediator-scan API through the Scan app #2039
Conversation
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
Signed-off-by: Divam <[email protected]>
@@ -0,0 +1,55 @@ | |||
-- Table 1: scan_verdict_store |
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.
nitpick: I would remove the table 1:
and table 2:
from these comments
|
||
-- Index for efficient querying by migration, domain, and record time | ||
create index scan_verdict_mi_di_rt on scan_verdict_store (migration_id, domain_id, record_time); | ||
create index scan_verdict_di_mi_rt on scan_verdict_store (domain_id, migration_id, record_time); |
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.
in which case is this index needed, why does scan_verdict_mi_di_rt
not suffice?
required: true | ||
schema: | ||
type: string | ||
- name: "lossless" |
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.
This should not be used (was part of a deprecated version of updates, /v0/updates), please follow example of /v2/updates/{update_id}
instead
description: | | ||
Returns the event with the given update_id. | ||
parameters: | ||
- name: "event_id" |
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.
Rather use update_id, since event_id also has a meaning in this API, but is used for something else
)(extracted: TraceContext): Future[ScanResource.GetEventByIdResponse] = { | ||
implicit val tc = extracted | ||
withSpan(s"$workflowId.getEventById") { _ => _ => | ||
val encoding = if (lossless.getOrElse(false)) { |
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.
do this like getUpdateByIdV2 instead
WIP branch with implementation for new scan-app endpoint
/v0/events
to provide data from mediator-scan API.Fixes #1564
Pull Request Checklist
Cluster Testing
/cluster_test
on this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_test
on this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n
, and mention issues worked on using#n
Merge Guidelines