implement multi platform api support#2185
Conversation
tharindu1st
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika and
tharikaGitHub
as code owners
Dependency Validation ResultsDependency name: github.com/knadh/koanf/v2 Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/wso2/api-platform/sdk/core Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/kelseyhightower/envconfig Next Steps
|
|
Warning Review limit reached
More reviews will be available in 6 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR integrates an Sequence DiagramsequenceDiagram
participant wsManager
participant EventDispatcher
participant EventHub
participant GatewayEventsService
participant WebSocketConn
Note over wsManager,EventDispatcher: Gateway connects
wsManager->>EventDispatcher: onConnect(gatewayID)
EventDispatcher->>EventHub: Register + Subscribe(gatewayID)
EventHub-->>EventDispatcher: subscription channel
Note over GatewayEventsService,EventHub: Event published
GatewayEventsService->>EventHub: PublishEvent(eventhub.Event)
EventHub->>EventDispatcher: event via subscription channel
EventDispatcher->>wsManager: GetConnections(gatewayID)
wsManager-->>EventDispatcher: []Connection
EventDispatcher->>WebSocketConn: Send(EventData)
Note over wsManager,EventDispatcher: Gateway disconnects
wsManager->>EventDispatcher: onDisconnect(gatewayID)
EventDispatcher->>EventHub: Unsubscribe(gatewayID)
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
9de7aa0 to
43b5ace
Compare
Dependency Validation Results |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
43b5ace to
7412a7c
Compare
Dependency Validation Results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/src/config/config.go`:
- Around line 63-79: Add a new validation step that rejects zero or negative
durations for the EventHub config: implement validateEventHubConfig(e EventHub)
error that returns an error if any of e.PollInterval, e.CleanupInterval, or
e.RetentionPeriod is <= 0 (with clear field-specific messages), and wire it into
GetConfig() alongside validateDeploymentsConfig so GetConfig() fails fast on
invalid EVENT_HUB_* values before server.go calls eventhub.New(...). Ensure the
error is propagated the same way other validation errors are (same error
type/format) so callers receive a consistent failure.
In `@platform-api/src/internal/server/server.go`:
- Around line 537-544: Start() currently only performs full cleanup in the
signal branch and never shuts down s.wsManager, so move the teardown logic into
a single helper (e.g., s.teardown or deferable function) and call it on every
exit path: when ListenAndServeTLS returns an error, in the signal-case branch,
and on normal shutdown. The helper should call httpServer.Shutdown(shutdownCtx)
(log errors), s.wsManager.Shutdown(), s.dispatcher.Shutdown(), and
s.eventHub.Close() (log errors) in a safe order and ensure it’s invoked exactly
once from Start()’s return paths.
In `@platform-api/src/internal/service/gateway_events.go`:
- Around line 298-307: The actionForEventType function currently maps ".created"
and ".deployed" to "CREATE" and ".deleted" to "DELETE", but misses ".undeployed"
so those events fall through to "UPDATE"; update actionForEventType to treat
event types ending with ".undeployed" as "DELETE" (e.g., add a case using
strings.HasSuffix(eventType, ".undeployed") that returns "DELETE" or include
".undeployed" alongside ".deleted") so the normalized action field correctly
reflects delete/undeploy operations.
In `@platform-api/src/internal/websocket/manager.go`:
- Around line 151-157: The connection hook invocations (m.onConnect /
m.onDisconnect) must not be dispatched in detached goroutines because that
allows out-of-order execution; change all call sites that do "go
m.onConnect(...)" or "go m.onDisconnect(...)" to call the hooks synchronously
immediately after the Manager updates its map/state (i.e., invoke
m.onConnect(gatewayID) and m.onDisconnect(gatewayID) inline, still keeping the
comment that callbacks must not depend on manager locks), or alternatively
implement per-gateway serialization (a small per-gateway mutex/serial queue) and
call the hooks through that serializer instead of with "go". Ensure the
inline/serialized call happens after the state change that made the
connect/disconnect effective so dispatcher sees consistent ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05aea41e-a877-4e3a-bff5-dc9fb7633485
⛔ Files ignored due to path filters (1)
platform-api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
kubernetes/helm/event-gateway-helm-chart/templates/event-gateway/controller/deployment.yamlkubernetes/helm/event-gateway-helm-chart/values.yamlkubernetes/helm/gateway-helm-chart/templates/gateway/controller/deployment.yamlkubernetes/helm/gateway-helm-chart/values.yamlplatform-api/src/config/config.goplatform-api/src/go.modplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/server/server.goplatform-api/src/internal/service/event_dispatcher.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/websocket/manager.go
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
7412a7c to
3b4e81c
Compare
Dependency Validation Results |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
3b4e81c to
a18f150
Compare
Dependency Validation Results |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
a18f150 to
4708466
Compare
Dependency Validation Results |
1 similar comment
Dependency Validation Results |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/src/internal/websocket/manager.go`:
- Around line 87-89: The comment describing the connection lifecycle hooks
(onConnect and onDisconnect) incorrectly states they are "called asynchronously"
when the actual implementation invokes them synchronously. Update the comment
block to replace "asynchronously" with "synchronously" to accurately reflect the
actual behavior of these hooks.
- Around line 151-157: The SetConnectionHooks method writes to the m.onConnect
and m.onDisconnect fields without synchronization while Register and Unregister
read these fields concurrently, creating a potential data race. Document this
constraint since current usage is safe (SetConnectionHooks is called only once
during initialization before Server.Start). Add a note to the SetConnectionHooks
function comment explaining that it must be called only during initialization
before the gateway accepts connections. Additionally, add documentation comments
to the m.onConnect and m.onDisconnect struct fields indicating that
SetConnectionHooks must be called only during initialization to avoid concurrent
access issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6eefc776-3a68-480e-9bbf-4b4862948584
⛔ Files ignored due to path filters (1)
platform-api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
platform-api/src/config/config.goplatform-api/src/config/default_config.goplatform-api/src/go.modplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/server/server.goplatform-api/src/internal/service/event_dispatcher.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/websocket/manager.go
🚧 Files skipped from review as they are similar to previous changes (6)
- platform-api/src/go.mod
- platform-api/src/internal/database/schema.postgres.sql
- platform-api/src/internal/service/event_dispatcher.go
- platform-api/src/internal/service/gateway_events.go
- platform-api/src/internal/database/schema.sql
- platform-api/src/internal/server/server.go
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
4708466 to
3b9e67e
Compare
Dependency Validation Results |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
67c26af to
6db1e9f
Compare
Dependency Validation Results |
1 similar comment
Dependency Validation Results |
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
6db1e9f to
a4efa7e
Compare
Dependency Validation Results |
1 similar comment
Dependency Validation Results |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/service/gateway_events.go (1)
244-296:⚠️ Potential issue | 🟠 MajorExtract entity identifiers from payload to set EntityID instead of using gatewayID.
The broadcastEventWithUserID function sets EntityID to gatewayID for all event types. However, each event model (DeploymentEvent, LLMProviderDeploymentEvent, etc.) contains the specific entity identifier (ApiId, ProviderId, ProxyId). This is inconsistent with the rest of the codebase, where EntityID represents the affected resource, not the container.
To align with codebase patterns and improve event semantics for downstream consumers, extract the entity identifier from the payload based on event type and set EntityID accordingly:
- For API-related events: use ApiId from the payload
- For LLM provider/proxy events: use ProviderId/ProxyId from the payload
- Similar mapping for other event types
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platform-api/src/internal/service/gateway_events.go` around lines 244 - 296, In the broadcastEventWithUserID function, the EntityID field in the eventhub.Event struct is currently hardcoded to gatewayID for all event types. Instead, extract the actual entity identifier from the payload parameter based on the eventType and use that for EntityID. Create a helper function or add logic that uses type assertion to extract the appropriate field (such as ApiId, ProviderId, or ProxyId) from the payload object based on the eventType, then set EntityID to this extracted value instead of gatewayID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@platform-api/src/internal/service/gateway_events.go`:
- Around line 244-296: In the broadcastEventWithUserID function, the EntityID
field in the eventhub.Event struct is currently hardcoded to gatewayID for all
event types. Instead, extract the actual entity identifier from the payload
parameter based on the eventType and use that for EntityID. Create a helper
function or add logic that uses type assertion to extract the appropriate field
(such as ApiId, ProviderId, or ProxyId) from the payload object based on the
eventType, then set EntityID to this extracted value instead of gatewayID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dee31f90-1772-4b29-925f-03d452a3a5ef
⛔ Files ignored due to path filters (1)
platform-api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
common/eventhub/eventhub_test.gocommon/eventhub/sqlbackend.gocommon/eventhub/topic.goplatform-api/src/config/config.goplatform-api/src/config/default_config.goplatform-api/src/go.modplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/server/server.goplatform-api/src/internal/service/event_dispatcher.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/websocket/manager.go
✅ Files skipped from review due to trivial changes (1)
- platform-api/src/go.mod
🚧 Files skipped from review as they are similar to previous changes (8)
- platform-api/src/internal/database/schema.sqlite.sql
- platform-api/src/config/default_config.go
- platform-api/src/internal/database/schema.postgres.sql
- platform-api/src/internal/websocket/manager.go
- platform-api/src/internal/database/schema.sql
- platform-api/src/config/config.go
- platform-api/src/internal/service/event_dispatcher.go
- platform-api/src/internal/server/server.go
tharindu1st
force-pushed
the
platform-api-high-availability
branch
from
a4efa7e to
002595e
Compare
Dependency Validation Results |
2 participants
implement multi platform api support