Salesforce impl: REST client, GraphQL client and gRPC implementation - #1
Salesforce impl: REST client, GraphQL client and gRPC implementation#1ness-david-dedu wants to merge 5 commits into
Conversation
| log: mgr.Logger(), | ||
| res: mgr, | ||
| req: req, | ||
| binLogCache: cacheResource, |
There was a problem hiding this comment.
It would be good to move the fields exposed as configuration options to a dedicated, exported config struct. See the Oracle CDC component for example.
| return nil, err | ||
| } | ||
|
|
||
| cdcReplayPreset, err := conf.FieldString("cdc_replay_preset") |
There was a problem hiding this comment.
Consider moving these duplicate string literals used for the config definition and parsing to a dedicated constant to reduce the chance of them getting out of sync. See the Oracle CDC component for example.
We also tend to follow a pattern of prefixing them with an abbreviation of the component name (so sfiCdcReplayPreset for instance (where sfi equals Sales Force Input)
| if err != nil { | ||
| return nil, err | ||
| } | ||
| s.log.Debugf("Fetching from Salesforce.. Input: %s", string(inputMsg)) |
There was a problem hiding this comment.
I'm not sure we need this. I can imagine this getting quite noisy and the potential to leak sensitive data?
| if err != nil { | ||
| return nil, err | ||
| } | ||
| //if err := license.CheckRunningEnterprise(mgr); err != nil { |
There was a problem hiding this comment.
Did you mean to commit this change?
| // limitations under the License. | ||
| // https://github.com/redpanda-data/connect/blob/main/licenses/rcl.md | ||
|
|
||
| // salesforce_helper.go provides helpers for making HTTP requests |
There was a problem hiding this comment.
Is all of this documentation intentional?
There was a problem hiding this comment.
updated the documentation
There was a problem hiding this comment.
Should return ctx.Err() here instead of context.Canceled.
| // Try loading new format first | ||
| raw, err := s.accessCache(ctx, "sf_state") | ||
| if err != nil { | ||
| return ProcessorState{}, nil |
There was a problem hiding this comment.
Should we be logging this error or being more intentional about it to ensure we don't skip important errors? One option might be for the caller to make this decision based on the type or error returned?
The same applies to line 514 as well.
| // Copyright 2026 Redpanda Data, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. |
There was a problem hiding this comment.
This license also needs updating to the enterprise license.
| } | ||
|
|
||
| // WithBackoff configures the reconnection backoff parameters. | ||
| func WithBackoff(base, max time.Duration, maxAttempts int) ClientOption { |
There was a problem hiding this comment.
A change was recently merged into main to add composable HTTP types. At some point it may be worth switching to these if easy enough.
There was a problem hiding this comment.
will be done at a later date
2 participants
No description provided.