Extend metadata to include targets for route_to_stream #24584
Conversation
gally47
left a comment
gally47
left a comment
There was a problem hiding this comment.
Tested it, Frontend LGTM.
kodjo-anipah
left a comment
kodjo-anipah
left a comment
There was a problem hiding this comment.
Left some comments.
I know its not part of the scope in terms of rules but a pipeline can be connected to a stream and since we are displaying that on the stream routing page like that we need to get those connections from PipelineStreamConnectionsService
|
|
||
| @JsonProperty(FIELD_ROUTING_RULES) | ||
| // Maps rule ID to IDs of routed stream | ||
| public abstract Map<String, Set<String>> routingRules(); |
There was a problem hiding this comment.
I think we should rename both variables so it's clear what they are. e.g. streamsByRuleId and routedStreamTitlesById
| if (args.getPreComputedValue(RouteToStream.ID_ARG) != null) { | ||
| String streamId = args.getPreComputedValue(RouteToStream.ID_ARG).toString(); | ||
| try { | ||
| Stream stream = streamService.load(streamId); |
There was a problem hiding this comment.
we can use streamService.streamTitleFromCache()
| schema = @Schema(allowableValues = {"asc", "desc"})) | ||
| @DefaultValue(DEFAULT_SORT_DIRECTION) @QueryParam("order") SortOrder order) { | ||
|
|
||
| // Pagination is primarily for UX purposes - OK to fetch all and then paginate in memory |
There was a problem hiding this comment.
I don't think we should do that. Pagination should run on the backend and against MongoDB even if that means that we can offer limited search and filtering. Also pagination is not being used in the frontend?
There was a problem hiding this comment.
Yes, the FE should use pagination. And we want to use the standard paginated entity table, so the response needs to conform to that pattern.
The problem is that the table entities do not correspond to mongo documents. A single entry in the pipeline_processor_rules_meta collection may map to multiple table entries. This is handled in the buildResponse method and is not exactly trivial.
If we want to perform pagination via mongo, I think we would need to modify the pipeline_processor_rules_meta collection so that it maps 1:1 to the UI table. The UI dictating the DB schema feels backward; especially since this collection is used for multiple purposes.
There was a problem hiding this comment.
Not sure about copying everything into that collection or if we can extend how the query works in the backend but i think frontend gets no filter and searches if it means doing that in memory. so having a data table that can only filter on one coloumn is what it is , or not?
There was a problem hiding this comment.
Yes, it's not pretty. It needs to be paginated because the space in the UI is very limited. But we don't expect to ever have a very large number of entries here, so filtering and searching is not that important.
That said, we already have at least one instance of in-memory pagination (collections). We could re-use that code for filtering and searching if needed.
There was a problem hiding this comment.
Pagination is being used in the frontend. In the screenshot there are no entries, so then there is no pagination.
There was a problem hiding this comment.
vs:
not talking about how the search doesn't do anything for me. but since we only filter on the pipline we don't need to filter in memory, but suit yourself
There was a problem hiding this comment.
If we want to perform pagination via mongo, I think we would need to modify the pipeline_processor_rules_meta collection so that it maps 1:1 to the UI table. The UI dictating the DB schema feels backward; especially since this collection is used for multiple purposes.
@patrickmann Is this something we could feasibly get done in this cooldown? Would the search bar work differently in this case?
/nocl
Description
See issue for context.
Motivation and Context
Resolves #24516
How Has This Been Tested?
Local testing
Note that no meta-data is generated for rules that are not included in any pipeline!
Screenshots (if appropriate):
Types of changes
Checklist: