Skip to content
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

Snowflake domain name with .aws appended after region code #544

Closed
ldsingh00 opened this issue Sep 4, 2020 · 4 comments
Closed

Snowflake domain name with .aws appended after region code #544

ldsingh00 opened this issue Sep 4, 2020 · 4 comments
Assignees
Labels

Comments

@ldsingh00
Copy link

ldsingh00 commented Sep 4, 2020

With the new snowflake changes https://docs.snowflake.com/en/user-guide/jdbc-configure.html
the region id parameter is being deprecated and account names have .aws/.azure for few regions appended to the end. So if I just pass the URI it pick only the first secrtion as account name and omits .aws/.azure component to it.

has account => (
    is      => 'ro',
    isa     => Str,
    lazy    => 1,
    default => sub {
        my $self = shift;
        if (my $host = $self->uri->host) {
            # <account_name>.<region_id>.snowflakecomputing.com
            $host =~ s/[.].+//;
            return $host;
        }
        return $self->_def_acct;  
@theory
Copy link
Collaborator

theory commented Sep 6, 2020

Ah, this applies only to the URL parsing? I guess it should leave the region and cloud platform in the account name, now? I think the code currently considers the region as part of the domain name. Would have to do some fiddling to make sure it switches to considering region and cloud platform as part of the account code. Cleaner overall, but kind of a PITA to change.

@theory theory self-assigned this Sep 6, 2020
@theory theory added the engine label Sep 6, 2020
@theory
Copy link
Collaborator

theory commented Sep 6, 2020

Does this fix the issue for you? I'll put it in a PR, too.

--- a/lib/App/Sqitch/Engine/snowflake.pm
+++ b/lib/App/Sqitch/Engine/snowflake.pm
@@ -114,8 +115,15 @@ sub _def_user {
 
 sub _def_pass { $ENV{SNOWSQL_PWD} || shift->_snowcfg->{password} }
 sub _def_acct {
-    return $ENV{SNOWSQL_ACCOUNT} || shift->_snowcfg->{accountname}
+    my $acct = $ENV{SNOWSQL_ACCOUNT} || $_[0]->_snowcfg->{accountname}
         || hurl engine => __('Cannot determine Snowflake account name');
+
+    # XXX Region is deprecated as a separate value, because the acount name may now be
+    # <account_name>.<region_id>.<cloud_platform>
+    # https://docs.snowflake.com/en/user-guide/snowsql-start.html#a-accountname
+    # Remove from here down and just return on the line above once Snowflake removes it.
+    my $region = $ENV{SNOWSQL_REGION} || $_[0]->_snowcfg->{region} or return $acct;
+    return "$acct.$region";
 }
 
 has account => (
@@ -125,8 +133,8 @@ has account => (
     default => sub {
         my $self = shift;
         if (my $host = $self->uri->host) {
-            # <account_name>.<region_id>.snowflakecomputing.com
-            $host =~ s/[.].+//;
+            # <account_name>.<region_id>.<cloud_platform>.snowflakecomputing.com
+            $host =~ s/[.]snowflakecomputing[.]com$//;
             return $host;
         }
         return $self->_def_acct;
@@ -136,16 +144,12 @@ has account => (
 sub _host {
     my ($self, $uri) = @_;
     if (my $host = $uri->host) {
-        # Allow host to just be account name or account + region.
         return $host if $host =~ /\.snowflakecomputing\.com$/;
         return $host . ".snowflakecomputing.com";
     }
+    # XXX SNOWSQL_HOST is deprecated; remove it once Snowflake removes it.
     return $ENV{SNOWSQL_HOST} if $ENV{SNOWSQL_HOST};
-    return join '.', (
-        $self->_def_acct,
-        (grep { $_ } $ENV{SNOWSQL_REGION} || $self->_snowcfg->{region} || ()),
-        'snowflakecomputing.com',
-    );
+    return $self->_def_acct . '.snowflakecomputing.com';
 }
 
 has warehouse => (

@ldsingh00
Copy link
Author

Yes just the URI section. Dropping region from code fixes it.

@theory
Copy link
Collaborator

theory commented Sep 12, 2020

So the patch fixes it for you, then?

@theory theory closed this as completed in f17e5a4 Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants