Skip to content

Commit bccb53f

Browse files
committed
refactor(clickhouse): remove credsProvider field, simplify createStagingStore
credsProvider (*ClickHouseS3Credentials) was stored on the connector but never read — all access goes through the StagingStore interface now. Removing it also eliminates the utils import from clickhouse.go.
1 parent 4286c43 commit bccb53f

2 files changed

Lines changed: 13 additions & 30 deletions

File tree

flow/connectors/clickhouse/clickhouse.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"go.temporal.io/sdk/log"
2222

2323
metadataStore "github.com/PeerDB-io/peerdb/flow/connectors/external_metadata"
24-
"github.com/PeerDB-io/peerdb/flow/connectors/utils"
2524
"github.com/PeerDB-io/peerdb/flow/generated/protos"
2625
"github.com/PeerDB-io/peerdb/flow/internal"
2726
peerdb_clickhouse "github.com/PeerDB-io/peerdb/flow/pkg/clickhouse"
@@ -33,9 +32,8 @@ type ClickHouseConnector struct {
3332
*metadataStore.PostgresMetadata
3433
database clickhouse.Conn
3534
logger log.Logger
36-
Config *protos.ClickhouseConfig
37-
credsProvider *utils.ClickHouseS3Credentials
38-
staging StagingStore
35+
Config *protos.ClickhouseConfig
36+
staging StagingStore
3937
chVersion *clickhouseproto.Version
4038
}
4139

@@ -61,7 +59,7 @@ func NewClickHouseConnector(
6159
return nil, fmt.Errorf("failed to get ClickHouse version: %w", err)
6260
}
6361

64-
staging, credsProvider, err := createStagingStore(ctx, env, config, clickHouseVersion.Version)
62+
staging, err := createStagingStore(ctx, env, config, clickHouseVersion.Version)
6563
if err != nil {
6664
return nil, err
6765
}
@@ -71,7 +69,6 @@ func NewClickHouseConnector(
7169
PostgresMetadata: pgMetadata,
7270
Config: config,
7371
logger: logger,
74-
credsProvider: credsProvider,
7572
staging: staging,
7673
chVersion: &clickHouseVersion.Version,
7774
}, nil

flow/connectors/clickhouse/staging.go

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,25 @@ import (
1616
"github.com/PeerDB-io/peerdb/flow/shared"
1717
)
1818

19-
// createStagingStore builds a StagingStore and (for S3) the legacy ClickHouseS3Credentials.
20-
// The credsProvider return is non-nil only for S3 and is kept for backward compatibility
21-
// with the session-token version check in createS3StagingStore.
2219
func createStagingStore(
2320
ctx context.Context,
2421
env map[string]string,
2522
config *protos.ClickhouseConfig,
2623
chVersion clickhouseproto.Version,
27-
) (StagingStore, *utils.ClickHouseS3Credentials, error) {
24+
) (StagingStore, error) {
2825
provider, err := internal.PeerDBClickHouseStagingProvider(ctx, env)
2926
if err != nil {
30-
return nil, nil, fmt.Errorf("failed to get staging provider: %w", err)
27+
return nil, fmt.Errorf("failed to get staging provider: %w", err)
3128
}
3229

3330
bucketName, err := internal.PeerDBClickHouseStagingBucketName(ctx, env)
3431
if err != nil {
35-
return nil, nil, fmt.Errorf("failed to get staging bucket name: %w", err)
32+
return nil, fmt.Errorf("failed to get staging bucket name: %w", err)
3633
}
3734

3835
switch strings.ToLower(provider) {
3936
case "gcs":
40-
store, err := newGCSStagingStore(ctx, bucketName)
41-
return store, nil, err
37+
return newGCSStagingStore(ctx, bucketName)
4238
default:
4339
return newS3StagingStoreFromConfig(ctx, config, bucketName, chVersion)
4440
}
@@ -49,7 +45,7 @@ func newS3StagingStoreFromConfig(
4945
config *protos.ClickhouseConfig,
5046
unifiedBucketName string,
5147
chVersion clickhouseproto.Version,
52-
) (StagingStore, *utils.ClickHouseS3Credentials, error) {
48+
) (StagingStore, error) {
5349
var awsConfig utils.PeerAWSCredentials
5450
var awsBucketPath string
5551
if config.S3 != nil {
@@ -69,12 +65,12 @@ func newS3StagingStoreFromConfig(
6965

7066
credentialsProvider, err := utils.GetAWSCredentialsProvider(ctx, "clickhouse", awsConfig)
7167
if err != nil {
72-
return nil, nil, err
68+
return nil, err
7369
}
7470

7571
if awsBucketPath == "" {
7672
if unifiedBucketName == "" {
77-
return nil, nil, errors.New("PeerDB ClickHouse Bucket Name not set")
73+
return nil, errors.New("PeerDB ClickHouse Bucket Name not set")
7874
}
7975
deploymentUID := internal.PeerDBDeploymentUID()
8076
flowName, _ := ctx.Value(shared.FlowNameKey).(string)
@@ -86,31 +82,21 @@ func newS3StagingStoreFromConfig(
8682
// https://github.com/ClickHouse/ClickHouse/issues/61230
8783
credentials, err := credentialsProvider.Retrieve(ctx)
8884
if err != nil {
89-
return nil, nil, err
85+
return nil, err
9086
}
9187
if credentials.AWS.SessionToken != "" {
9288
if !clickhouseproto.CheckMinVersion(
9389
clickhouseproto.Version{Major: 24, Minor: 3, Patch: 1},
9490
chVersion,
9591
) {
96-
return nil, nil, fmt.Errorf(
92+
return nil, fmt.Errorf(
9793
"provide S3 Transient Stage details explicitly or upgrade to ClickHouse version >= 24.3.1, current version is %s. %s",
9894
chVersion,
9995
"You can also contact PeerDB support for implicit S3 stage setup for older versions of ClickHouse.")
10096
}
10197
}
10298

103-
creds := &utils.ClickHouseS3Credentials{
104-
Provider: credentialsProvider,
105-
BucketPath: awsBucketPath,
106-
}
107-
108-
store, err := newS3StagingStore(awsBucketPath, credentialsProvider)
109-
if err != nil {
110-
return nil, nil, err
111-
}
112-
113-
return store, creds, nil
99+
return newS3StagingStore(awsBucketPath, credentialsProvider)
114100
}
115101

116102
func newGCSStagingStore(ctx context.Context, bucketName string) (StagingStore, error) {

0 commit comments

Comments
 (0)