Refactor credential types to support multiple cloud storage backends#584
Refactor credential types to support multiple cloud storage backends#584sfc-gh-pczajka wants to merge 1 commit intomainfrom
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
There was a problem hiding this comment.
Pull request overview
Refactors Snowflake stage credential handling in sf_core to support multiple cloud storage backends by introducing typed location/credential variants and updating query-response parsing and file-transfer entry points accordingly.
Changes:
- Added
LocationTypeandCloudCredentialsto represent cloud backends and credentials at the type level. - Extended
StageInfowithlocation_typeandpresigned_url, and updated Snowflake query response parsing to populate them. - Updated upload/download dispatch to branch by
location_type, returningUnsupportedStorageTypefor non-S3 backends (for now).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
sf_core/src/rest/snowflake/query_response.rs |
Parses locationType/presignedUrl and builds typed file_manager::StageInfo credentials based on backend. |
sf_core/src/file_manager/types.rs |
Introduces LocationType, CloudCredentials, and extends StageInfo to carry backend + optional presigned URL. |
sf_core/src/file_manager/mod.rs |
Routes upload/download by location_type and adds UnsupportedStorageType error variant. |
sf_core/src/file_manager/file_transfer.rs |
Updates S3 client creation to pull credentials from the CloudCredentials enum. |
You can also share your feedback on Copilot code review. Take the survey.
655d560 to
d67aefa
Compare
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Replace the S3-only Credentials struct with a CloudCredentials enum that supports S3 and GCS credential types at the type level. Add LocationType enum and extend StageInfo with presigned_url field. Update query response parsing to build credentials based on location type. GCS and Azure operations return UnsupportedStorageType for now. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d67aefa to
3291a41
Compare
There was a problem hiding this comment.
Pull request overview
Refactors Snowflake file-transfer stage metadata and credential modeling so the type system can represent multiple cloud storage backends (S3 today, GCS/Azure as stubs), and wires the new enums through query response parsing and the file manager.
Changes:
- Introduces
LocationTypeandCloudCredentials(S3/GCS) and extendsStageInfowithlocation_typeandpresigned_url. - Updates query response parsing to derive
file_manager::StageInfobased onlocationTypeand construct backend-specific credentials. - Updates S3 transfer code to consume
CloudCredentials::S3and adds explicit “missing S3 credentials” errors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
sf_core/src/rest/snowflake/query_response.rs |
Parses locationType/presignedUrl and constructs backend-specific StageInfo + credentials. |
sf_core/src/file_manager/types.rs |
Adds LocationType, CloudCredentials, and new StageInfo fields to represent multiple backends. |
sf_core/src/file_manager/s3_transfer.rs |
Refactors S3 transfer to require S3 credentials variant and improves Snafu context usage. |
sf_core/src/file_manager/mod.rs |
Routes upload/download by LocationType, returning unsupported errors for non-S3 backends. |
Comments suppressed due to low confidence (2)
sf_core/src/file_manager/s3_transfer.rs:180
create_s3_clientmatches onstage_info.credsby value, which moves the enum out of a borrowed&StageInfoand should not compile. Match on a reference instead (e.g.,match &stage_info.creds { CloudCredentials::S3 { ... } => ... }) or destructure vialet CloudCredentials::S3 { .. } = &stage_info.credsto avoid moving.
sf_core/src/file_manager/s3_transfer.rs:228- The
From<S3CredentialError>impls constructMissingS3CredentialswithLocation::default(), which loses the actual call-site#[snafu(implicit)]location and makes diagnostics much less useful. Prefer returningUploadFileError/DownloadFileErrordirectly fromcreate_s3_client(or use a Snafu context helper) so the implicitLocationis captured at the point of failure.
bf60f36 to
3291a41
Compare

Summary
Credentialsstruct withCloudCredentialsenum supporting S3 and GCS at the type levelLocationTypeenum and extendStageInfowithpresigned_urlfieldlocationTypefrom serverUnsupportedStorageTypeerror (implementation in follow-up PR)Part 1 of 2 — type system refactoring only, no new cloud backend functionality.
Test plan
🤖 Generated with Claude Code