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

Add hostname support to snowflake #39471

Merged
merged 6 commits into from Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,15 @@
import styled from "@emotion/styled";

import Button from "metabase/core/components/Button";
import { color } from "metabase/lib/colors";

export const SectionButton = styled(Button)`
color: ${color("brand")};
padding: 0;
border: none;
border-radius: 0;

&:hover {
background-color: transparent;
}
`;
@@ -0,0 +1,32 @@
import { useField } from "formik";
import { useCallback } from "react";
import { t } from "ttag";

import FormField from "metabase/core/components/FormField";

import { SectionButton } from "./DatabaseHostameSectionField.styled";

export interface DatabaseHostnameSectionFieldProps {
name: string;
}

const DatabaseHostnameSectionField = ({
name,
}: DatabaseHostnameSectionFieldProps): JSX.Element => {
const [{ value }, , { setValue }] = useField(name);

const handleClick = useCallback(() => {
setValue(!value);
}, [value, setValue]);

return (
<FormField>
<SectionButton type="button" onClick={handleClick}>
{value ? t`Use hostname` : t`Use account name`}
</SectionButton>
</FormField>
);
};

// eslint-disable-next-line import/no-default-export -- deprecated usage
export default DatabaseHostnameSectionField;
@@ -0,0 +1,2 @@
// eslint-disable-next-line import/no-default-export -- deprecated usage
export { default } from "./DatabaseHostameSectionField";
4 changes: 4 additions & 0 deletions frontend/src/metabase/databases/constants.tsx
Expand Up @@ -6,6 +6,7 @@ import DatabaseAuthCodeDescription from "./components/DatabaseAuthCodeDescriptio
import DatabaseCacheScheduleField from "./components/DatabaseCacheScheduleField";
import DatabaseClientIdDescription from "./components/DatabaseClientIdDescription";
import DatabaseConnectionSectionField from "./components/DatabaseConnectionSectionField";
import DatabaseHostnameSectionField from "./components/DatabaseHostnameSectionField";
import DatabaseScheduleToggleField from "./components/DatabaseScheduleToggleField";
import DatabaseSshDescription from "./components/DatabaseSshDescription";
import DatabaseSslKeyDescription from "./components/DatabaseSslKeyDescription";
Expand Down Expand Up @@ -104,6 +105,9 @@ export const FIELD_OVERRIDES: Record<string, EngineFieldOverride> = {
"use-conn-uri": {
type: DatabaseConnectionSectionField,
},
"use-hostname": {
type: DatabaseHostnameSectionField,
},
"let-user-control-scheduling": {
type: DatabaseScheduleToggleField,
},
Expand Down
9 changes: 9 additions & 0 deletions modules/drivers/snowflake/resources/metabase-plugin.yaml
Expand Up @@ -8,11 +8,20 @@ driver:
lazy-load: true
parent: sql-jdbc
connection-properties:
- name: use-hostname
type: section
default: false
- merge:
- host
- visible-if:
use-hostname: true
- name: account
display-name: Account name
helper-text: Enter your Account ID with the region that your Snowflake cluster is running on e.g. "xxxxxxxx.us-east-2.aws". Some regions don't have this suffix.
placeholder: xxxxxxxx.us-east-2.aws
required: true
visible-if:
use-hostname: false
- user
- password
- name: private-key
Expand Down
9 changes: 7 additions & 2 deletions modules/drivers/snowflake/src/metabase/driver/snowflake.clj
Expand Up @@ -123,15 +123,20 @@
(str "\"" (str/replace raw-name "\"" "\"\"") "\"")))

(defmethod sql-jdbc.conn/connection-details->spec :snowflake
[_ {:keys [account additional-options], :as details}]
[_ {:keys [account additional-options host use-hostname], :as details}]
(when (get "week_start" (sql-jdbc.common/additional-options->map additional-options :url))
(log/warn (trs "You should not set WEEK_START in Snowflake connection options; this might lead to incorrect results. Set the Start of Week Setting instead.")))
(let [upcase-not-nil (fn [s] (when s (u/upper-case-en s)))]
;; it appears to be the case that their JDBC driver ignores `db` -- see my bug report at
;; https://support.snowflake.net/s/question/0D50Z00008WTOMCSA5/
(-> (merge {:classname "net.snowflake.client.jdbc.SnowflakeDriver"
:subprotocol "snowflake"
:subname (str "//" account ".snowflakecomputing.com/")
;; see https://github.com/metabase/metabase/issues/22133
:subname (let [base-url (if (and use-hostname (string? host) (not (str/blank? host)))
(cond-> host
(not= (last host) \/) (str "/"))
(str account ".snowflakecomputing.com/"))]
(str "//" base-url ))
:client_metadata_request_use_connection_ctx true
:ssl true
;; keep open connections open indefinitely instead of closing them. See #9674 and
Expand Down
Expand Up @@ -87,13 +87,29 @@
:engine :snowflake
:private-key-creator-id 3
:user "SNOWFLAKE_DEVELOPER"
:private-key-created-at "2024-01-05T19:10:30.861839Z"}]
(testing "Database name is quoted iff quoting is requested (#27856)"
:private-key-created-at "2024-01-05T19:10:30.861839Z"
:host ""}]
(testing "Database name is quoted if quoting is requested (#27856)"
(are [quote? result] (=? {:db result}
(let [details (assoc details :quote-db-name quote?)]
(sql-jdbc.conn/connection-details->spec :snowflake details)))
true "\"v3_sample-dataset\""
false "v3_sample-dataset"))))
false "v3_sample-dataset"))
(testing "Subname is replaced if hostname is provided (#22133)"
(are [use-hostname alternative-host expected-subname] (=? expected-subname
(:subname (let [details (-> details
(assoc :host alternative-host)
(assoc :use-hostname use-hostname))]
(sql-jdbc.conn/connection-details->spec :snowflake details))))
true nil "//ls10467.us-east-2.aws.snowflakecomputing.com/"
true "" "//ls10467.us-east-2.aws.snowflakecomputing.com/"
true " " "//ls10467.us-east-2.aws.snowflakecomputing.com/"
true "snowflake.example.com/" "//snowflake.example.com/"
true "snowflake.example.com" "//snowflake.example.com/"
false nil "//ls10467.us-east-2.aws.snowflakecomputing.com/"
false "" "//ls10467.us-east-2.aws.snowflakecomputing.com/"
false "snowflake.example.com/" "//ls10467.us-east-2.aws.snowflakecomputing.com/"
false "snowflake.example.com" "//ls10467.us-east-2.aws.snowflakecomputing.com/"))))

(deftest ^:parallel ddl-statements-test
(testing "make sure we didn't break the code that is used to generate DDL statements when we add new test datasets"
Expand Down