Conversation
63a3615 to
b49cc86
Compare
sophia-bq
reviewed
Feb 27, 2026
common/lib/host_list_provider/monitoring/monitoring_host_list_provider.ts
Show resolved
Hide resolved
common/lib/host_list_provider/monitoring/monitoring_host_list_provider.ts
Show resolved
Hide resolved
3eb5328 to
fac01e6
Compare
|
|
||
| import { TopologyUtils } from "./topology_utils"; | ||
|
|
||
| export class RdsTopologyUtils extends TopologyUtils { |
Contributor
There was a problem hiding this comment.
I believe the idea of having these topology utils is to have clear separation and roles on dialect and host provider where:
- dialect would be focused on sql queries that is specific to the db language.
- topology util: owns executing
- host list provider manages the cache of host lists and is only exposed to topology util and not database language specific.
Pros of doing this from what I see is:
- Removes duplicated parsing logic
- Easier test isolation
- Central point for host construction logic in topology utils
- Dialect doesn't need to care about host construction
Contributor
Author
There was a problem hiding this comment.
Discussed this offline, it is worth giving this a thought and try to decouple more of the topology processing logic from database dialects. Will create a ticket for this in the backlog.
This PR will keep using the existing logic where database dialect handles most of the query execution and processing.
fac01e6 to
45da8ac
Compare
sophia-bq
approved these changes
Mar 3, 2026
faviansamatha
approved these changes
Mar 3, 2026
This file contains hidden or 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
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.
Summary
Refactor methods for querying and processing topology information into a TopologyUtils class.
Description
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.