Skip to content

Commit

Permalink
Add hostname support to snowflake (#39471)
Browse files Browse the repository at this point in the history
* Add alternative hostname support to snowflake

* move hostname support out of additional details

* Update modules/drivers/snowflake/src/metabase/driver/snowflake.clj

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

* Update modules/drivers/snowflake/src/metabase/driver/snowflake.clj

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

* Update modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

* opt-in

---------

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
  • Loading branch information
vieux and metamben committed Mar 15, 2024
1 parent 2ae6ade commit 9c44af4
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 5 deletions.
@@ -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
22 changes: 19 additions & 3 deletions modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj
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

0 comments on commit 9c44af4

Please sign in to comment.