Skip to content

Conversation

@pmsanford
Copy link

Hello! I've made some changes to add basic support for using Oauth tokens for authentication in the library. The motivation for this change is to support connecting using the token (and other configuration variables) provided by Snowpark Container Services to connect to Snowflake on the internal network. This change does not provide any facility for performing the oauth flow since in SPCS the token is just provided in an environment variable.

I've tested this manually in SPCS as well as running the existing tests against a local dev instance using the host/port parameters, but I haven't added any automated tests because I don't know which features your test instance has access to. I'm happy to add some, though, depending on what's available and your tolerance for spend.

I've also fixed a bug that prevents the SESSION_EXPIRED error from ever being bubbled up - instead, previously, if a session expired, the error was always a json decoding error.

Thanks for this connector, it's made experimentation and prototyping a lot easier.

Disclaimer: I work for Snowflake, but this contribution is made in my personal capacity and not on behalf of Snowflake in any way.

@pmsanford
Copy link
Author

Woops, fixed formatting issue.

Copy link
Member

@kenkoooo kenkoooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve noticed that the host, port, and protocol are usually configured together. To simplify URL configuration, I recommend providing an enum so users can choose between specifying the account components or supplying a full URL directly. For example:

pub struct SnowflakeSession {
    // …
    connection: SnowflakeConnectionConfig,
}

pub enum SnowflakeConnectionConfig {
    /// Configure using account identifier
    Account(String),
    /// Configure using container URL
    Url(Url),
}

@pmsanford
Copy link
Author

I’ve noticed that the host, port, and protocol are usually configured together. To simplify URL configuration, I recommend providing an enum so users can choose between specifying the account components or supplying a full URL directly. For example:

The account name is needed elsewhere even when the hostname is provided, so unfortunately it won't work to make them mutually exclusive.

My intent for choosing 3 separate configuration options was so they align with the variables provided in the SPCS environment (eg SNOWFLAKE_HOST on its own), and with the way the Python connector accepts these separately. I'm happy to refactor to instead add an access_url field that combines protocol, host, and port similar to the nodejs connector (though it also has a host option separately), what do you think?

Another direction could be to have something like

trait SnowflakeConnectionConfig {
  fn get_base_url(&self) -> String;
}

and then instead of adding fields to SnowflakeClientConfig, creating a separate struct like SnowflakeExtendedClientConfig and implement the trait for both - that's a more invasive change, but it has the benefit of being non-breaking.

@pmsanford
Copy link
Author

@kenkoooo I switched to a different approach that uses with_address similar to the existing with_proxy - and so doesn't modify the config object or any of the public API (other than adding with_address). Let me know if you think this is better.

sfc-gh-mdrach
sfc-gh-mdrach approved these changes Sep 3, 2025
Copy link

@mdrach mdrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this PR together. This change is critical for basic SPCS usage, as setup pulls from env variables like in this example.

Accepting both account and host seems consistent with other tools like snowflake cli.

cc @kenkoooo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants