Conversation
- Update baton-sdk to v0.7.10 - Create pkg/config package with generated configuration - Update main.go to use config.RunConnector API - Update connector to use V2 interface - Update Makefile for config generation and lambda support - Update GitHub workflows Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughUpgrades multiple dependencies, adds a generated Sendgrid config type with reflection-based accessors, refactors package config (removing viper and ValidateConfig), and rewrites the connector constructor to accept a config object and build the SendGrid client internally. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Config as Config (pkg/config)
participant ConnectorCtor as Connector.New
participant ClientPkg as client.NewClient
participant Connector as Connector (builder)
rect rgba(100,149,237,0.5)
Caller->>ConnectorCtor: call New(ctx, cfg.Sendgrid, opts)
end
rect rgba(144,238,144,0.5)
ConnectorCtor->>Config: read API key, region, ignoreSubusers
ConnectorCtor->>ClientPkg: NewClient(apiKey, baseURL)
ClientPkg-->>ConnectorCtor: client or error
ConnectorCtor->>Connector: construct Connector with client & cache
ConnectorCtor-->>Caller: return ConnectorBuilderV2, opts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/connector/connector.go (1)
57-64:⚠️ Potential issue | 🔴 CriticalCritical:
*Connectordoes not implementconnectorbuilder.ConnectorBuilderV2interface.The
ResourceSyncersmethod returns[]connectorbuilder.ResourceSyncer(lines 57-64), butConnectorBuilderV2requires[]connectorbuilder.ResourceSyncerV2. Additionally, all resource builder implementations (teammates.go, subuser.go, scopes.go, teammate_invitations.go) must be updated to implementResourceSyncerV2Limitedwith the new method signatures:
- Change
List,Entitlements, andGrantsmethod signatures from using*pagination.Tokentoresource.SyncOpAttrs- Update return types from
(string, annotations.Annotations, error)to(*resource.SyncOpResults, error)Also applies to: 122-151
🧹 Nitpick comments (2)
pkg/connector/connector.go (2)
21-23: Consider removing unused error variable.
ErrSendgridClientNotProvidedappears to be unused now that theNewfunction creates the client internally rather than accepting it as a parameter.🧹 Proposed removal
-var ( - ErrSendgridClientNotProvided = errors.New("sendgrid client not provided") -)Also remove the
errorsimport if no other errors are defined in this file.import ( "context" - "errors" "io"
131-139: Region handling logs warning for empty string default value.When
sendgridRegionis empty (e.g., if the config field wasn't set despite having a default), the switch falls through to default and logs a warning. SinceSendGridRegionFieldhasWithDefaultValue("global"), this should typically be fine, but the warning message "invalid sendgrid region" may be misleading if the region is simply empty rather than invalid.💡 Consider handling empty string explicitly
switch sendgridRegion { case "eu": baseUrl = client.SendGridEUBaseUrl - case "global": + case "global", "": baseUrl = client.SendGridBaseUrl default: baseUrl = client.SendGridBaseUrl l.Warn("invalid sendgrid region, using the default global URL", zap.String("region", sendgridRegion)) }
- Use RunConnector with WithDefaultCapabilitiesConnectorBuilder - Update connector.New signature (already correct) - Update ResourceSyncers to return ResourceSyncerV2 - Fix workflow to generate capabilities_and_config.yaml - Run go mod tidy -v && go mod vendor Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/connector/connector.go (1)
57-63:⚠️ Potential issue | 🔴 CriticalBuild failure: Builders do not implement
connectorbuilder.ResourceSyncerV2interface correctly.The builders (e.g.,
*teammateBuilder,*scopeBuilder, etc.) have incorrect method signatures that don't matchResourceSyncerV2. Specifically:
- Parameter mismatch: Methods use
*pagination.TokenbutResourceSyncerV2requiresresource.SyncOpAttrs- Return type mismatch: Methods return
(result, string, annotations.Annotations, error)butResourceSyncerV2requires(result, *resource.SyncOpResults, error)Update the
List,Entitlements, andGrantsmethod signatures in all builder files (teammates.go,scopes.go,subuser.go,teammate_invitations.go) to match theResourceSyncerV2Limitedinterface.Additionally, the
opts *cli.ConnectorOptsparameter in theNew()function (line 122) is unused and can be removed.
🧹 Nitpick comments (3)
pkg/connector/connector.go (3)
122-122: Unused parameteropts *cli.ConnectorOpts.The
optsparameter is accepted but never referenced in the function body. If this is required by theConnectorBuilderV2constructor signature convention, consider adding a comment to clarify. Otherwise, if it's intended for future use, ensure it gets wired in or remove it to avoid confusion.
21-23:ErrSendgridClientNotProvidedis now dead code.Since the constructor now creates the client internally rather than accepting it as a parameter, this error is no longer used. Consider removing it.
🧹 Proposed fix
-var ( - ErrSendgridClientNotProvided = errors.New("sendgrid client not provided") -)Also remove the
"errors"import at line 5 if no longer needed elsewhere in this file.
131-139: Region handling logic looks good, minor suggestion on empty string case.The region validation with a default fallback is appropriate. However, when the region is an empty string (unset), the warning "invalid sendgrid region" might be misleading. Consider distinguishing between an empty/unset region and an actually invalid value.
💡 Optional: Distinguish empty from invalid region
switch sendgridRegion { case "eu": baseUrl = client.SendGridEUBaseUrl - case "global": + case "global", "": baseUrl = client.SendGridBaseUrl default: baseUrl = client.SendGridBaseUrl l.Warn("invalid sendgrid region, using the default global URL", zap.String("region", sendgridRegion)) }
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace pagination.Token with rs.SyncOpAttrs parameter and return *rs.SyncOpResults instead of string/annotations in List, Entitlements, and Grants methods across all resource syncers (teammates, subuser, scopes, teammate_invitations). This aligns with the V2 SDK pattern for resource synchronization. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix pagination token pointer usage in 5 locations: - Changed opts.PageToken == nil check to opts.PageToken.Token == "" - Changed opts.PageToken to &opts.PageToken when passing to client methods - Update main.go to use WithDefaultCapabilitiesConnectorBuilderV2 - Add baton-sendgrid binary to .gitignore Resolves compilation errors in: - pkg/connector/scopes.go:34 - pkg/connector/subuser.go:35 - pkg/connector/teammate_invitations.go:27 - pkg/connector/teammates.go:32,71 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 participant
Containerizes the connector following baton-databricks#35 and baton-contentful#48.
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit
Chores
Refactor
New Features