-
Notifications
You must be signed in to change notification settings - Fork 0
Update sdk to v0.3.10 and update config #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update refactors the PostgreSQL connector's configuration fields to use a multi-line format with explicit display names and metadata, including marking sensitive fields as secret. It also updates the baton-sdk dependency version and adjusts how the viper dependency is declared in go.mod. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Config
participant Field
User->>Config: Initialize PostgreSQL configuration
Config->>Field: Define DSN (secret, display name)
Config->>Field: Define Schemas (display name)
Config->>Field: Define IncludeColumns (display name)
Config->>Field: Define IncludeLargeObjects (display name)
Config->>Field: Define SyncAllDatabases (display name)
Config->>Config: Attach connector display name and help URL
Config-->>User: Return enriched configuration schema
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for "github.com/conductorone/baton-sdk/pkg/field"" ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/config/config.go (2)
21-25: Specify default for boolean fieldFor clarity and consistency, add an explicit default to the
includeColumnsfield:@@ line 21-25 includeColumns = field.BoolField( "include-columns", field.WithDisplayName("Include Columns"), field.WithDescription("Include column privileges when syncing. This can result in large amounts of data"), + field.WithDefaultValue(false), )
26-30: Specify default for boolean fieldSimilarly, declare an explicit default for
includeLargeObjects:@@ line 26-30 includeLargeObjects = field.BoolField( "include-large-objects", field.WithDisplayName("Include Large Objects"), field.WithDescription("Include large objects when syncing. This can result in large amounts of data"), + field.WithDefaultValue(false), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (59)
go.sumis excluded by!**/*.sumand included by nonevendor/github.com/conductorone/baton-sdk/internal/connector/connector.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/config/v1/config.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/config/v1/config.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/annotation_raw_id.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/annotation_raw_id.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/connector.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/event_feed.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/event_feed.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/event_feed_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connector/v2/resource_grpc.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connectorapi/baton/v1/baton.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/connectorapi/baton/v1/baton.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/reader/v2/sync.pb.gois excluded by!**/*.pb.go,!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pb/c1/reader/v2/sync.pb.validate.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/cli/commands.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/cli/lambda_server__added.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/config/config.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/connectorbuilder/connectorbuilder.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/connectorrunner/runner.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/connectorstore/connectorstore.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/assets.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/c1file.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/diff.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/entitlements.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/grants.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/resouce_types.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/resources.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/sql_helpers.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/dotc1z/sync_runs.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/field/default_relationships.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/field/defaults.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/field/fields.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/field/marshal.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/field/struct.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/retry/retry.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sdk/empty_connector.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sdk/version.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sync/state.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/sync/syncer.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/synccompactor/compactor.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/synccompactor/naive/naive.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/synccompactor/naive/naive_unroll.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/c1api/full_sync.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/c1api/manager.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/local/compactor.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/local/differ.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/local/event_feed.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/local/syncer.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/tasks/tasks.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/types/entitlement/entitlement.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/types/tasks/tasks.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/types/types.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/pagination.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/transport.gois excluded by!vendor/**and included by nonevendor/github.com/conductorone/baton-sdk/pkg/uhttp/wrapper.gois excluded by!vendor/**and included by nonevendor/modules.txtis excluded by!vendor/**and included by none
📒 Files selected for processing (2)
go.mod(2 hunks)pkg/config/config.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: go-lint
🔇 Additional comments (6)
go.mod (2)
6-6: Ensure module compatibility and tidy dependencies.The
github.com/conductorone/baton-sdkupdate tov0.3.10is intentional—please rungo mod tidy(and updatego.sum) to ensure no breaking changes or unresolved module issues remain.
93-93:✅ Verification successful
Verify removal of direct viper dependency.
github.com/spf13/viperwas shifted to an indirect requirement. Confirm no remaining direct imports rely on viper by searching the codebase.
🏁 Script executed:
#!/bin/bash # Verify no direct import of viper remains rg -n 'import .*viper' -n .Length of output: 29
🏁 Script executed:
#!/bin/bash # Search for direct import of viper rg -n '"github.com/spf13/viper"' --glob '*.go' # Search for any usage of viper package identifiers (e.g., viper.Optional) rg -n 'viper\.' --glob '*.go'Length of output: 80
Removal of direct viper dependency confirmed
No occurrences of
github.com/spf13/viperimports or anyviper.usages were found across the codebase’s.gofiles. The shift to an indirect requirement ingo.modis safe.pkg/config/config.go (4)
8-14: Mark DSN as secret and requiredUsage of
field.WithIsSecret(true)along withWithRequired(true)properly secures the DSN. Looks correct.
15-20: Provide default schemasSetting
WithDefaultValue([]string{"public"})ensures a sensible default. Implementation is clear.
31-36: Default value on syncAllDatabases is explicitIncluding
WithDefaultValue(false)makes this consistent with other boolean fields. Implementation is correct.
40-50: Connector metadata initialization is clearThe addition of
WithConnectorDisplayName("PostgreSQL")andWithHelpUrl("/docs/baton/postgresql")improves UX. Configuration fields are ordered logically.
|
Postgres is a service mode (self-hosted) connector so it shouldn't need container support |
😆 ..... oops, well... I guess that would've been better to know when I did the first round of updates for this. Shall I just close this PR then @laurenleach ? |
hmm I would argue we probably want at least some of these changes - the sdk change, outputting baton config and capabilities, and also the updates to use the new shared workflow for releases should still apply. we'll be updating the template repo to match this moving forwards at least |
Summary by CodeRabbit
New Features
Refactor