feat(plugin-redis): add Sentinel connection mode (#1021)#1215
Open
tonghs wants to merge 3 commits intoTableProApp:mainfrom
Open
feat(plugin-redis): add Sentinel connection mode (#1021)#1215tonghs wants to merge 3 commits intoTableProApp:mainfrom
tonghs wants to merge 3 commits intoTableProApp:mainfrom
Conversation
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
Phase 1 of #1021: adds Redis Sentinel connection mode to the Redis plugin. Users with Sentinel-fronted HA deployments can now point TablePro at the Sentinel quorum instead of a single underlying Redis node, and reconnects automatically follow master failover.
Cluster mode (Phase 2 of the issue) is not in this PR.
Closes Phase 1 of #1021.
What this PR does
Sentinel resolution path (
Plugins/RedisDriverPlugin/)RedisSentinelResolver— pure Swift, iterates Sentinel nodes in order, returns first node's master address; throws.masterUnknown(vs.allSentinelsUnreachable) when at least one Sentinel responded but didn't know the masterHiredisSentinelTransport— production transport, opens a short-lived hiredis connection per Sentinel, runsSENTINEL get-master-addr-by-name, applies optional Sentinel-plane AUTH separately from data-plane AUTHRedisPluginDriver.connect()now branches on a new `redisMode` field; sentinel mode runs the resolver, then opens the existing single-node connection to the resolved master. The resolver re-runs on every reconnect (no cached address), soConnectionHealthMonitorfailover Just WorksConnection form schema (
RedisPlugin.swift+PluginMetadataRegistry+RegistryDefaults.swift)visibleWhento hide in Single modeTwo framework fixes uncovered while wiring this up (split into separate commits)
fix(plugins):PluginMetadataSnapshot.withBrandingwas overwriting the plugin'sconnectionwith the registry default's. When a plugin added newadditionalConnectionFieldsto its source without also updatingRegistryDefaults, the new fields were silently dropped. Only preserves visual branding now (displayName / iconName / brandColorHex)fix(connection-form):hasHostListFieldis now visibility-aware (was returning true whenever any hostList field existed in metadata, even when hidden);AuthPaneViewModel.isFieldVisiblenow falls back to the network pane's field values so cross-sectionvisibleWhenrules work;HostListFieldRowplaceholder usedText("hostname:\(defaultPort)")which let SwiftUI render the Int as a localized number with thousand separators (hostname:6,379) — switched toText(verbatim:). Also affects MongoDBTest plan
RedisSentinelResolverTests— 22 cases covering iteration semantics, reply parsing, hostList parsingPluginMetadataRegistryBrandingTests— 2 cases locking in thewithBrandingfixNotes for the maintainer
UI is not my strong suit. I tried to follow existing connection-form conventions (visibleWhen, hostList, dropdown), and reordered Sentinel fields a few times based on RedisInsight as reference. If you'd rather restructure the form (e.g., segmented control for Mode, distinct "Sentinel" subsection, different field naming), feel free to take over that part. I'd rather you redo the UI than ship something off-brand.
Localization: I only verified English and Chinese (zh-Hans). I added Turkish and Vietnamese translations on a best-effort basis (no native review), so please have them sanity-checked by speakers if you have access. The 12 new strings live in
TablePro/Resources/Localizable.xcstrings.No native protocol changes: the plugin's
additionalConnectionFieldsarray is the source of truth, butRegistryDefaults.swiftmirrors it (because built-in plugins are lazily instantiated, so the form reads the registry default before the plugin loads). That's an existing convention in this codebase; if you'd like, a follow-up could collapse the dual source by lazy-activating the driver on first metadata read.Open question in the issue I didn't pick up: Q3 (hiredis vs hiredis-cluster) is irrelevant for Sentinel; it'll come back when Phase 2 (Cluster) starts.