feat: Init salesforce sync #36
Conversation
|
""" WalkthroughA Salesforce sync integration was added to the Ctrlplane CLI, enabling synchronization of Salesforce Accounts and Opportunities as resources. This includes new command implementations, utility functions for Salesforce client initialization, SOQL querying with pagination, resource transformation, and upsertion to Ctrlplane. A comprehensive README documents usage. The Makefile was updated for linting and formatting, and Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Salesforce
participant Ctrlplane
User->>CLI: Run 'sync salesforce accounts' or 'sync salesforce opportunities'
CLI->>Salesforce: Authenticate and query records (SOQL with pagination)
Salesforce-->>CLI: Return Account/Opportunity records
CLI->>CLI: Transform records to Ctrlplane resource format (apply metadata mappings)
CLI->>Ctrlplane: Upsert resources via API
Ctrlplane-->>CLI: Confirm upsert
CLI-->>User: Display sync results
Poem
""" ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
Makefile (1)
26-26: Consider adding an "all" target as suggested by static analysis.While not strictly required, adding an
alltarget is a common Makefile convention that can serve as the default target.+all: build + .PHONY: build install test clean lint formatcmd/ctrlc/root/sync/salesforce/README.md (1)
219-264: Consider documenting rate limiting and error handlingThe implementation details section would benefit from mentioning:
- Salesforce API rate limits and how the sync handles them
- Error handling and retry strategies for transient failures
- Recommended sync frequency to avoid hitting API limits
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go (1)
225-229: Handle potentially missing custom fields gracefullyThe default metadata mappings include custom fields (Region__c, Annual_Revenue__c, Tier__c, Customer_Health__c) that might not exist in all Salesforce instances. Consider documenting that these are example custom fields or implement logic to check field existence.
cmd/ctrlc/root/sync/salesforce/common/util.go (1)
158-329: Consider breaking down QuerySalesforceObject for better maintainabilityThis function is quite complex (~170 lines) and handles multiple concerns. Consider extracting some logic into helper functions:
- Field discovery and logging (lines 194-223)
- Query building with pagination (lines 232-255)
- Batch processing (lines 282-322)
This would improve readability and testability.
cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go (1)
225-231: Limited date format handlingThe date parsing only handles the "2006-01-02" format. Salesforce might return dates in different formats depending on the field type or configuration.
Consider handling additional date formats or documenting the expected format:
-if t, err := time.Parse("2006-01-02", opportunity.CloseDate); err == nil { +// Try multiple date formats +for _, format := range []string{"2006-01-02", time.RFC3339, "2006-01-02T15:04:05.000+0000"} { + if t, err := time.Parse(format, opportunity.CloseDate); err == nil { + closeDateFormatted = t.Format(time.RFC3339) + break + } +} +if closeDateFormatted == "" { closeDateFormatted = opportunity.CloseDate -} else { - closeDateFormatted = opportunity.CloseDate }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefile(1 hunks)cmd/ctrlc/root/sync/salesforce/README.md(1 hunks)cmd/ctrlc/root/sync/salesforce/accounts/accounts.go(1 hunks)cmd/ctrlc/root/sync/salesforce/common/client.go(1 hunks)cmd/ctrlc/root/sync/salesforce/common/util.go(1 hunks)cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go(1 hunks)cmd/ctrlc/root/sync/salesforce/salesforce.go(1 hunks)cmd/ctrlc/root/sync/sync.go(2 hunks)go.mod(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
cmd/ctrlc/root/sync/sync.go (1)
cmd/ctrlc/root/sync/salesforce/salesforce.go (1)
NewSalesforceCmd(10-25)
cmd/ctrlc/root/sync/salesforce/salesforce.go (2)
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go (1)
NewSalesforceAccountsCmd(92-152)cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go (1)
NewSalesforceOpportunitiesCmd(88-140)
🪛 checkmake (0.2.2)
Makefile
[warning] 26-26: Missing required phony target "all"
(minphony)
🔇 Additional comments (13)
Makefile (1)
20-25: LGTM! Good addition of code quality targets.The new
lintandformattargets will help maintain consistent code quality across the expanding codebase, especially with the new Salesforce sync functionality.cmd/ctrlc/root/sync/sync.go (2)
11-11: LGTM! Follows established integration pattern.The Salesforce sync import is properly placed alphabetically among the other sync integrations.
42-42: LGTM! Consistent command registration.The Salesforce command registration follows the same pattern as other sync commands like GitHub and Kubernetes.
cmd/ctrlc/root/sync/salesforce/salesforce.go (1)
1-26: LGTM! Well-structured command implementation.The Salesforce command follows established patterns:
- Clean package structure
- Proper use of Cobra command framework
- Clear examples using heredoc
- Appropriate subcommand registration
The implementation provides a solid foundation for the Salesforce sync functionality.
cmd/ctrlc/root/sync/salesforce/common/client.go (3)
11-21: LGTM! Proper error handling and client initialization.The Salesforce client initialization follows good practices:
- Proper error wrapping with context
- Clean function signature
- Appropriate use of the salesforce library
23-41: LGTM! Robust credential validation with environment variable fallback.The validation function properly:
- Checks for environment variable fallbacks
- Validates all required credentials
- Provides clear error messages
- Returns appropriate validation function for Cobra
7-7: Third-Party Salesforce Client Library VerifiedThe Go library
github.com/k-capehart/go-salesforce/v2has been reviewed and meets our maintenance and security requirements:
- Actively maintained with commits as recent as March 2025 and a healthy issue lifecycle (open, discussed, and closed).
- Dependabot integrations and prompt updates to key dependencies (e.g.,
github.com/golang-jwt/jwt/v5) ensure robust security posture.- No known unresolved vulnerabilities or advisories reported in its GitHub security panel or issue tracker.
- It remains the most actively maintained Go wrapper for Salesforce; no better Go alternatives are currently available.
No further action required.
go.mod (4)
13-13: LGTM! Appropriate dependency addition for heredoc support.The
github.com/MakeNowJust/heredocv1.0.0 dependency is needed for the command examples in the Salesforce commands.
30-30: LGTM! Core dependency for Salesforce integration.The
github.com/k-capehart/go-salesforce/v2v2.5.2 dependency is essential for the Salesforce API integration.
43-43: LGTM! Kubernetes dependency update.The upgrade of
k8s.io/apimachineryfrom v0.32.1 to v0.33.2 appears to be a routine maintenance update.
30-30: All new Go module dependencies are secure and up-to-dateVerified via GitHub’s Security Vulnerabilities API:
- github.com/k-capehart/go-salesforce/v2 v2.5.2: no known advisories
- github.com/forcedotcom/go-soql (as pinned in go.mod): no known advisories
- github.com/go-viper/mapstructure/v2 v2.3.0: advisory “May Leak Sensitive Information…” affected < 2.3.0; v2.3.0 is the first patched release
- github.com/jszwec/csvutil (as pinned in go.mod): no known advisories
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go (2)
15-90: Well-structured dynamic field handlingThe Account struct and custom JSON unmarshaling implementation effectively handle both known and dynamic Salesforce fields. This pattern allows for flexibility when dealing with custom Salesforce fields.
92-152: Excellent command structure with comprehensive examplesThe command setup follows Cobra best practices with clear examples, proper flag organization, and validation. The heredoc examples are particularly helpful for users.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
cmd/ctrlc/root/sync/salesforce/common/util.go (6)
140-140: Consider using fixed precision for float formatting consistency.While the current implementation using
strconv.FormatFloat(val, 'f', -1, 64)is better than the previous%gformat, the-1precision still uses the minimum number of digits necessary, which may lead to inconsistent string representations for the same logical value.Consider using a fixed precision for more consistent formatting:
-return strconv.FormatFloat(val, 'f', -1, 64) +return strconv.FormatFloat(val, 'f', 2, 64) // Or appropriate precision for your use case
159-160: Consider refactoring function signature to reduce parameter count.The
QuerySalesforceObjectfunction has 8 parameters, which makes it difficult to use and maintain. Consider using a configuration struct or builder pattern.Consider refactoring to use a configuration struct:
type QueryConfig struct { ObjectName string Limit int ListAllFields bool AdditionalFields []string WhereClause string } func QuerySalesforceObject(ctx context.Context, sf *salesforce.Salesforce, config QueryConfig, target interface{}) error { // implementation }
92-92: Improve type safety for custom field value conversion.Using
fmt.Sprintf("%v", value)may not handle all types correctly and could lead to unexpected string representations.Consider using a more type-safe approach:
-strValue := fmt.Sprintf("%v", value) +var strValue string +switch v := value.(type) { +case string: + strValue = v +case nil: + strValue = "" +default: + strValue = fmt.Sprintf("%v", v) +}
287-289: Enhance error handling with contextual information.When unmarshaling fails, it would be helpful to have more context about what data caused the failure for debugging purposes.
Consider adding debug logging for failed unmarshaling:
if err := json.Unmarshal(queryResult.Records, batchSlice.Addr().Interface()); err != nil { + log.Debug("Failed to unmarshal records", "error", err, "raw_data", string(queryResult.Records)) return fmt.Errorf("failed to unmarshal records: %w", err) }
326-328: Optimize slice truncation for large datasets.The current approach creates a new slice when truncating, which could be memory-inefficient for very large datasets.
Consider using slice reslicing instead of creating a new slice:
if limit > 0 && targetValue.Len() > limit { - targetValue.Set(targetValue.Slice(0, limit)) + // More efficient: just reslice without creating new backing array + targetValue.SetLen(limit) }
162-164: Improve error message specificity.The error message could be more descriptive about what type is expected and what was actually provided.
Make the error message more descriptive:
if targetValue.Kind() != reflect.Slice { - return fmt.Errorf("target must be a pointer to a slice") + return fmt.Errorf("target must be a pointer to a slice, got %T", target) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/ctrlc/root/sync/salesforce/README.md(1 hunks)cmd/ctrlc/root/sync/salesforce/common/util.go(1 hunks)cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/ctrlc/root/sync/salesforce/README.md
- cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker (linux/amd64)
🔇 Additional comments (2)
cmd/ctrlc/root/sync/salesforce/common/util.go (2)
1-432: Overall code quality assessment.The code is well-structured and handles the complex task of Salesforce synchronization effectively. The implementation demonstrates good practices like:
- Proper error handling and logging
- Use of reflection for dynamic field handling
- Pagination support for large datasets
- Custom field support through interfaces
The previous issues identified in past reviews have been appropriately addressed.
376-379: Add validation for required configuration values.The function retrieves configuration values from viper but doesn't validate that they're not empty, which could lead to runtime errors.
Add validation for required configuration values:
func UpsertToCtrlplane(ctx context.Context, resources []api.CreateResource, providerName string) error { apiURL := viper.GetString("url") apiKey := viper.GetString("api-key") workspaceId := viper.GetString("workspace") + + if apiURL == "" { + return fmt.Errorf("API URL is required") + } + if apiKey == "" { + return fmt.Errorf("API key is required") + } + if workspaceId == "" { + return fmt.Errorf("workspace ID is required") + }⛔ Skipped due to learnings
Learnt from: adityachoudhari26 PR: ctrlplanedev/cli#4 File: cmd/ctrlc/root/sync/terraform/terraform.go:35-45 Timestamp: 2025-01-15T17:46:32.343Z Learning: In the Ctrlplane CLI, the API client (`api.NewAPIKeyClientWithResponses`) handles invalid credentials internally by returning an error, making explicit validation of apiURL and apiKey unnecessary.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go (1)
129-141: Add validation for required Salesforce credentialsConsider validating that required Salesforce credentials are provided before attempting to initialize the client. This will provide clearer error messages to users.
domain := viper.GetString("salesforce-domain") consumerKey := viper.GetString("salesforce-consumer-key") consumerSecret := viper.GetString("salesforce-consumer-secret") +if domain == "" || consumerKey == "" || consumerSecret == "" { + return fmt.Errorf("salesforce-domain, salesforce-consumer-key, and salesforce-consumer-secret are required") +} + log.Info("Syncing Salesforce opportunities into Ctrlplane", "domain", domain)cmd/ctrlc/root/sync/salesforce/common/util.go (2)
18-30: Use url.Parse for more robust URL parsingConsider using the standard library's
url.Parsefor more reliable URL handling instead of manual string splitting.func GetSalesforceSubdomain(domain string) string { - subdomain := "salesforce" - if strings.HasPrefix(domain, "https://") || strings.HasPrefix(domain, "http://") { - parts := strings.Split(domain, "//") - if len(parts) > 1 { - hostParts := strings.Split(parts[1], ".") - if len(hostParts) > 0 { - subdomain = hostParts[0] - } - } - } - return subdomain + u, err := url.Parse(domain) + if err != nil || u.Host == "" { + return "salesforce" + } + + hostParts := strings.Split(u.Host, ".") + if len(hostParts) > 0 { + return hostParts[0] + } + return "salesforce" }
295-330: Solid implementation of Ctrlplane resource upsertionThe function correctly handles the two-step process of provider upsertion followed by resource setting, with appropriate error handling and logging.
Consider adding retry logic for transient network failures in future iterations to improve reliability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go(1 hunks)cmd/ctrlc/root/sync/salesforce/common/util.go(1 hunks)cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/ctrlc/root/sync/salesforce/accounts/accounts.go
🔇 Additional comments (6)
cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go (3)
19-83: Well-structured custom field handlingThe Opportunity struct and custom unmarshaling implementation effectively handle both standard and custom Salesforce fields. The use of an alias pattern to avoid infinite recursion is a good practice.
166-187: Clean implementation of opportunity processingThe function effectively leverages common utilities for querying and follows a clear pattern for transforming Salesforce data to Ctrlplane resources.
189-263: Comprehensive resource transformation with proper precision handlingThe transformation correctly handles date formatting, preserves floating-point precision using
-1format, and creates well-structured resources with both metadata and config fields. Good job addressing the previous precision concerns.cmd/ctrlc/root/sync/salesforce/common/util.go (3)
32-59: Robust field value extraction with proper reflection handlingThe function effectively handles both custom fields from the map and standard struct fields, with appropriate validation checks for field validity.
61-249: Well-implemented pagination with proper resource managementThe query implementation effectively handles:
- ID-based pagination to avoid SOQL OFFSET limitations
- Dynamic field extraction from struct tags
- Proper batch size management with limits
- Resource cleanup (addressing the previous defer-in-loop issue)
Good architectural choice for scalable data retrieval.
251-293: Clean implementation of field extraction utilitiesThe utilities effectively handle struct field extraction and custom field separation, enabling dynamic handling of Salesforce's flexible schema.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go (1)
182-208: Consider consistent nil handling in config valuesSimilar to the accounts implementation, the config map uses
fmt.Sprintf("%v", ...)which converts nil to "" string.
🧹 Nitpick comments (2)
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go (1)
144-183: Consider consistent nil handling in config valuesThe config map uses
fmt.Sprintf("%v", ...)for most values, which converts nil to "" string. This differs from the metadata handling which properly filters out nil values usingcommon.AddToMetadata.Consider creating a helper function or reusing the nil-checking logic to ensure consistent handling:
+func formatConfigValue(value any) string { + if value == nil { + return "" + } + str := fmt.Sprintf("%v", value) + if str == "<nil>" { + return "" + } + return str +} config := map[string]interface{}{ - "name": fmt.Sprintf("%v", account["Name"]), - "industry": fmt.Sprintf("%v", account["Industry"]), + "name": formatConfigValue(account["Name"]), + "industry": formatConfigValue(account["Industry"]), // ... apply to other string fields }Also note that numeric fields like
numberOfEmployees,BillingLatitude, andBillingLongitudeare stored as raw values (line 158, 166, 167), while converted to strings in metadata. Ensure this difference is intentional.cmd/ctrlc/root/sync/salesforce/common/util.go (1)
18-30: Use url.Parse for more robust URL parsingThe current string splitting approach could be fragile. Consider using the standard library's URL parser:
func GetSalesforceSubdomain(domain string) string { - subdomain := "salesforce" - if strings.HasPrefix(domain, "https://") || strings.HasPrefix(domain, "http://") { - parts := strings.Split(domain, "//") - if len(parts) > 1 { - hostParts := strings.Split(parts[1], ".") - if len(hostParts) > 0 { - subdomain = hostParts[0] - } - } - } - return subdomain + parsedURL, err := url.Parse(domain) + if err != nil || parsedURL.Host == "" { + return "salesforce" + } + hostParts := strings.Split(parsedURL.Host, ".") + if len(hostParts) > 0 { + return hostParts[0] + } + return "salesforce" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/ctrlc/root/sync/salesforce/README.md(1 hunks)cmd/ctrlc/root/sync/salesforce/accounts/accounts.go(1 hunks)cmd/ctrlc/root/sync/salesforce/common/util.go(1 hunks)cmd/ctrlc/root/sync/salesforce/opportunities/opportunities.go(1 hunks)cmd/ctrlc/root/sync/salesforce/salesforce.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cmd/ctrlc/root/sync/salesforce/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/ctrlc/root/sync/salesforce/salesforce.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/ctrlc/root/sync/salesforce/accounts/accounts.go (3)
cmd/ctrlc/root/sync/salesforce/common/client.go (1)
InitSalesforceClient(9-19)cmd/ctrlc/root/sync/salesforce/common/util.go (4)
GetSalesforceSubdomain(18-30)UpsertToCtrlplane(238-273)QuerySalesforceObject(33-84)AddToMetadata(229-236)internal/api/client.gen.go (1)
CreateResource(168-176)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker (linux/amd64)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/ctrlc/root/sync/salesforce/common/util.go (2)
18-30: Consider usingnet/urlpackage for more robust URL parsing.The current string-based parsing approach may not handle all URL edge cases correctly. Using the standard
net/urlpackage would be more reliable:func GetSalesforceSubdomain(domain string) string { - subdomain := "salesforce" - if strings.HasPrefix(domain, "https://") || strings.HasPrefix(domain, "http://") { - parts := strings.Split(domain, "//") - if len(parts) > 1 { - hostParts := strings.Split(parts[1], ".") - if len(hostParts) > 0 { - subdomain = hostParts[0] - } - } - } - return subdomain + // Add scheme if missing for proper URL parsing + if !strings.HasPrefix(domain, "http://") && !strings.HasPrefix(domain, "https://") { + domain = "https://" + domain + } + + parsedURL, err := url.Parse(domain) + if err != nil { + return "salesforce" // fallback on parse error + } + + hostParts := strings.Split(parsedURL.Hostname(), ".") + if len(hostParts) > 0 && hostParts[0] != "" { + return hostParts[0] + } + + return "salesforce" }
33-84: Consider using options pattern to reduce function complexity.The function has 7 parameters, making it difficult to use and maintain. Consider implementing an options pattern:
type QueryOptions struct { Limit int ListAllFields bool AdditionalFields []string WhereClause string } func QuerySalesforceObject(ctx context.Context, sf *salesforce.Salesforce, objectName string, target interface{}, opts QueryOptions) error { // implementation }This would make the function calls more readable and easier to extend in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/ctrlc/root/sync/salesforce/common/util.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: zacharyblasczyk
PR: ctrlplanedev/cli#36
File: cmd/ctrlc/root/sync/salesforce/common/util.go:0-0
Timestamp: 2025-07-16T21:29:15.789Z
Learning: In Salesforce sync code, the `lastId` parameter in `buildSOQL` function comes from previous Salesforce API responses via `getRecordId()`, making it safe from SOQL injection as it originates from Salesforce itself, not user input.
cmd/ctrlc/root/sync/salesforce/common/util.go (1)
Learnt from: zacharyblasczyk
PR: ctrlplanedev/cli#36
File: cmd/ctrlc/root/sync/salesforce/common/util.go:0-0
Timestamp: 2025-07-16T21:29:15.789Z
Learning: In Salesforce sync code, the `lastId` parameter in `buildSOQL` function comes from previous Salesforce API responses via `getRecordId()`, making it safe from SOQL injection as it originates from Salesforce itself, not user input.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker (linux/amd64)
🔇 Additional comments (7)
cmd/ctrlc/root/sync/salesforce/common/util.go (7)
86-114: LGTM! Well-implemented field discovery function.The function properly handles API requests, JSON parsing, and resource cleanup. The type assertions are safe and the error handling is comprehensive.
117-138: LGTM! Proper SOQL query construction with safe pagination.The function correctly constructs SOQL queries with proper condition handling and pagination. The
lastIdparameter is safely used as confirmed by previous reviews.
140-152: LGTM! Safe reflection-based ID extraction.The function properly validates reflection operations and handles edge cases with appropriate fallbacks.
154-198: LGTM! Efficient pagination implementation.The function correctly implements ID-based pagination with proper batch processing, limit handling, and reflection operations. The debug logging provides good visibility into the process.
201-227: LGTM! Proper query execution and result unmarshaling.The function correctly handles HTTP requests, URL escaping, JSON unmarshaling, and reflection operations with appropriate error handling.
229-236: LGTM! Clean metadata handling utility.The function properly filters out nil and empty values while converting to string representation for metadata storage.
238-273: LGTM! Complete resource upsert implementation.The function properly handles the full workflow of creating API clients, upserting providers, and setting resources with comprehensive error handling and logging.
Summary by CodeRabbit
New Features
Documentation
Chores