Skip to content

Commit 3ddcb26

Browse files
jeff303kulyk
andauthored
Add support for driver deprecation (metabase#17028)
# Backend changes Introducing new `superseded-by` property to plugin manifest YAML, which will indicate the driver that is to eventually replace this one (and will drive UI/UX behavior). If a driver declares this property, then it's considered to be deprecated in favor of the specified one. Adding top level `test_modules` directory (with the same structure as modules) for the sole purpose of module/plugin testing of YAML files, which will not be included with the driver build Updating `driver-plugin-manifest` to look for the new `test_modules` directory in addition to `modules`, when loading the driver manifest # Frontend changes Calculate `supersededBy` and supersedes maps from the "superseded-by" property for each engine Change the options for the engine field to use a function to dynamically show the legacy driver if allowed by rules (either the new driver is selected, or the legacy driver was already selected for an existing DB, or the driver is not superseded by anything) Add new `DriverWarning` component to show these warnings based on supersede status Co-authored-by: Anton Kulyk <[email protected]>
1 parent 33939b0 commit 3ddcb26

File tree

18 files changed

+276
-13
lines changed

18 files changed

+276
-13
lines changed

frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx

+10-4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import ActionButton from "metabase/components/ActionButton";
1515
import AddDatabaseHelpCard from "metabase/components/AddDatabaseHelpCard";
1616
import Button from "metabase/components/Button";
1717
import Breadcrumbs from "metabase/components/Breadcrumbs";
18+
import DriverWarning from "metabase/components/DriverWarning";
1819
import Radio from "metabase/components/Radio";
1920
import ModalWithTrigger from "metabase/components/ModalWithTrigger";
2021

@@ -210,15 +211,20 @@ export default class DatabaseEditApp extends Component {
210211
)}
211212
</LoadingAndErrorWrapper>
212213
</Box>
213-
{addingNewDatabase && (
214-
<Box>
214+
<Box>
215+
<DriverWarning
216+
engine={selectedEngine}
217+
ml={26}
218+
data-testid="database-setup-driver-warning"
219+
/>
220+
{addingNewDatabase && (
215221
<AddDatabaseHelpCard
216222
engine={selectedEngine}
217223
ml={26}
218224
data-testid="database-setup-help-card"
219225
/>
220-
</Box>
221-
)}
226+
)}
227+
</Box>
222228
</Flex>
223229
</div>
224230
</Box>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import React from "react";
2+
import PropTypes from "prop-types";
3+
4+
import { t } from "ttag";
5+
6+
import {
7+
allEngines,
8+
engineSupersedesMap,
9+
} from "metabase/entities/databases/forms";
10+
11+
import Warnings from "metabase/query_builder/components/Warnings";
12+
13+
import {
14+
CardContent,
15+
DriverWarningContainer,
16+
IconContainer,
17+
} from "./DriverWarning.styled";
18+
import ExternalLink from "metabase/components/ExternalLink";
19+
import MetabaseSettings from "metabase/lib/settings";
20+
21+
const propTypes = {
22+
engine: PropTypes.string.isRequired,
23+
};
24+
25+
const driverUpgradeHelpLink = MetabaseSettings.docsUrl(
26+
"administration-guide/01-managing-databases",
27+
);
28+
29+
function getSupersedesWarningContent(newDriver, supersedesDriver) {
30+
return (
31+
<div>
32+
<p className="text-medium m0">
33+
{t`This is our new ${
34+
allEngines[newDriver]["driver-name"]
35+
} driver, which is faster and more reliable.`}
36+
</p>
37+
<p>{t`The old driver has been deprecated and will be removed in the next release. If you really
38+
need to use it, you can select ${
39+
allEngines[supersedesDriver]["driver-name"]
40+
} now.`}</p>
41+
</div>
42+
);
43+
}
44+
45+
function getSupersededByWarningContent(engine) {
46+
return (
47+
<div>
48+
<p className="text-medium m0">
49+
{t`This driver has been deprecated and will be removed in the next release.`}
50+
</p>
51+
<p className="text-medium m0">
52+
{t`We recommend that you upgrade to the new ${
53+
allEngines[engine]["driver-name"]
54+
} driver, which is faster and more reliable.`}
55+
</p>
56+
<ExternalLink
57+
href={driverUpgradeHelpLink}
58+
className="text-brand text-bold"
59+
>
60+
{t`How to upgrade a driver`}
61+
</ExternalLink>
62+
</div>
63+
);
64+
}
65+
66+
function DriverWarning({ engine, ...props }) {
67+
const supersededBy = engineSupersedesMap["superseded_by"][engine];
68+
const supersedes = engineSupersedesMap["supersedes"][engine];
69+
70+
if (!supersedes && !supersededBy) {
71+
return null;
72+
}
73+
74+
const tooltipWarning = supersedes ? t`New driver` : t`Driver deprecated`;
75+
const warningContent = supersedes
76+
? getSupersedesWarningContent(engine, supersedes)
77+
: getSupersededByWarningContent(supersededBy);
78+
79+
return (
80+
<DriverWarningContainer p={2} {...props}>
81+
<IconContainer>
82+
<Warnings
83+
className="mx2 text-gold"
84+
warnings={[tooltipWarning]}
85+
size={20}
86+
/>
87+
</IconContainer>
88+
<CardContent flexDirection="column" justify="center" className="ml2">
89+
{warningContent}
90+
</CardContent>
91+
</DriverWarningContainer>
92+
);
93+
}
94+
95+
DriverWarning.propTypes = propTypes;
96+
97+
export default DriverWarning;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import styled from "styled-components";
2+
import { Flex } from "grid-styled";
3+
4+
export const CardContent = styled(Flex)``;
5+
6+
export const IconContainer = styled.div`
7+
display: flex;
8+
justify-content: center;
9+
align-items: center;
10+
`;
11+
12+
export const DriverWarningContainer = styled(Flex)`
13+
background-color: #f9fbfb;
14+
border-radius: 10px;
15+
width: 300px;
16+
margin-bottom: 8px;
17+
`;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { default } from "./DriverWarning";

frontend/src/metabase/entities/databases/forms.js

+42-2
Original file line numberDiff line numberDiff line change
@@ -278,21 +278,59 @@ function getEngineFormFields(engine, details, id) {
278278
});
279279
}
280280

281-
const ENGINE_OPTIONS = Object.entries(MetabaseSettings.get("engines") || {})
281+
const ENGINES = MetabaseSettings.get("engines", {});
282+
const ENGINE_OPTIONS = Object.entries(ENGINES)
282283
.map(([engine, info]) => ({
283284
name: info["driver-name"],
284285
value: engine,
285286
}))
286287
.sort((a, b) => a.name.localeCompare(b.name));
287288

289+
// use top level constant for engines so we only need to compute these maps once
290+
const ENGINE_SUPERSEDES_MAPS = Object.keys(ENGINES).reduce(
291+
(acc, engine) => {
292+
const newEngine = ENGINES[engine]["superseded-by"];
293+
if (newEngine) {
294+
acc.supersedes[newEngine] = engine;
295+
acc.superseded_by[engine] = newEngine;
296+
}
297+
return acc;
298+
},
299+
{ supersedes: {}, superseded_by: {} },
300+
);
301+
302+
/**
303+
* Returns the options to show in the engines selection widget. An engine is available to be selected if either
304+
* - it is not superseded by any other engine
305+
* - it is the selected engine (i.e. someone is already using it)
306+
* - it is superseded by some engine, which happens to be the currently selected one
307+
*
308+
* The idea behind this behavior is to only show someone a "legacy" driver if they have at least selected the one that
309+
* will replace it first, at which point they can "fall back" on the legacy one if needed.
310+
*
311+
* @param currentEngine the current (selected engine)
312+
* @returns the filtered engine options to be shown in the selection widget
313+
*/
314+
function getEngineOptions(currentEngine) {
315+
return ENGINE_OPTIONS.filter(engine => {
316+
const engineName = engine.value;
317+
const newDriver = ENGINE_SUPERSEDES_MAPS["superseded_by"][engineName];
318+
return (
319+
typeof newDriver === "undefined" ||
320+
newDriver === currentEngine ||
321+
engineName === currentEngine
322+
);
323+
});
324+
}
325+
288326
const forms = {
289327
details: {
290328
fields: ({ id, engine, details = {} } = {}) => [
291329
{
292330
name: "engine",
293331
title: t`Database type`,
294332
type: "select",
295-
options: ENGINE_OPTIONS,
333+
options: getEngineOptions(engine),
296334
placeholder: t`Select a database`,
297335
},
298336
{
@@ -382,3 +420,5 @@ const SCHEDULING_FIELDS = new Set([
382420
]);
383421

384422
export default forms;
423+
export const engineSupersedesMap = ENGINE_SUPERSEDES_MAPS;
424+
export const allEngines = ENGINES;

frontend/src/metabase/setup/components/Setup.jsx

+6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import MetabaseAnalytics from "metabase/lib/analytics";
88
import MetabaseSettings from "metabase/lib/settings";
99

1010
import AddDatabaseHelpCard from "metabase/components/AddDatabaseHelpCard";
11+
import DriverWarning from "metabase/components/DriverWarning";
1112
import ExternalLink from "metabase/components/ExternalLink";
1213
import LogoIcon from "metabase/components/LogoIcon";
1314
import NewsletterForm from "metabase/components/NewsletterForm";
@@ -203,6 +204,11 @@ export default class Setup extends Component {
203204
</div>
204205

205206
<AddDatabaseHelpCardHolder isVisible={isDatabaseHelpCardVisible}>
207+
<DriverWarning
208+
engine={selectedDatabaseEngine}
209+
ml={26}
210+
data-testid="database-setup-driver-warning"
211+
/>
206212
<AddDatabaseHelpCard
207213
engine={selectedDatabaseEngine}
208214
hasCircle={false}

src/metabase/driver.clj

+16
Original file line numberDiff line numberDiff line change
@@ -617,3 +617,19 @@
617617
[_ db-details]
618618
;; no normalization by default
619619
db-details)
620+
621+
(defmulti superseded-by
622+
"Returns the driver that supersedes the given `driver`. A non-nil return value means that the given `driver` is
623+
deprecated in Metabase and will eventually be replaced by the returned driver, in some future version (at which point
624+
any databases using it will be migrated to the new one).
625+
626+
This is currently only used on the frontend for the purpose of showing/hiding deprecated drivers. A driver can make
627+
use of this facility by adding a top-level `superseded-by` key to its plugin manifest YAML file, or (less preferred)
628+
overriding this multimethod directly."
629+
{:added "0.41.0" :arglists '([driver])}
630+
dispatch-on-uninitialized-driver
631+
:hierarchy #'hierarchy)
632+
633+
(defmethod superseded-by :default
634+
[_]
635+
nil)

src/metabase/driver/util.clj

+2-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@
100100
:when props]
101101
;; TODO - maybe we should rename `details-fields` -> `connection-properties` on the FE as well?
102102
[driver {:details-fields props
103-
:driver-name (driver/display-name driver)}])))
103+
:driver-name (driver/display-name driver)
104+
:superseded-by (driver/superseded-by driver)}])))
104105

105106
;;; +----------------------------------------------------------------------------------------------------------------+
106107
;;; | TLS Helpers |

src/metabase/plugins/initialize.clj

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
(@initialized-plugin-names plugin-name))
4646

4747
(s/defn init-plugin-with-info!
48-
"Initiaize plugin using parsed info from a plugin maifest. Returns truthy if plugin was successfully initialized;
48+
"Initialize plugin using parsed info from a plugin manifest. Returns truthy if plugin was successfully initialized;
4949
falsey otherwise."
5050
[info :- {:info {:name s/Str, :version s/Str, s/Keyword s/Any}
5151
s/Keyword s/Any}]

src/metabase/plugins/lazy_loaded_driver.clj

+3-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
"Register a basic shell of a Metabase driver using the information from its Metabase plugin"
6464
[{:keys [add-to-classpath!]
6565
init-steps :init
66+
superseded-by :superseded-by
6667
{driver-name :name, :keys [abstract display-name parent], :or {abstract false}, :as driver-info} :driver}]
6768
{:pre [(map? driver-info)]}
6869
(let [driver (keyword driver-name)
@@ -81,7 +82,8 @@
8182
(doseq [[^MultiFn multifn, f]
8283
{driver/initialize! (make-initialize! driver add-to-classpath! init-steps)
8384
driver/display-name (when display-name (constantly display-name))
84-
driver/connection-properties (constantly connection-props)}]
85+
driver/connection-properties (constantly connection-props)
86+
driver/superseded-by (constantly (keyword superseded-by))}]
8587
(when f
8688
(.addMethod multifn driver f)))
8789
;; finally, register the Metabase driver
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
(ns metabase.driver.driver-deprecation-test-legacy
2+
"Dummy driver for driver deprecation testing (legacy driver)"
3+
(:require [metabase.driver :as driver]))
4+
5+
(driver/register! :driver-deprecation-test-legacy, :parent :sql)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
(ns metabase.driver.driver-deprecation-test-new
2+
"Dummy driver for driver deprecation testing (new driver)"
3+
(:require [metabase.driver :as driver]))
4+
5+
(driver/register! :driver-deprecation-test-new, :parent :sql)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
(ns metabase.plugins.driver-deprecation-test
2+
(:require [clojure.test :refer :all]
3+
[metabase.models.setting :as setting]
4+
[metabase.test :as mt]
5+
[metabase.test.fixtures :as fixtures]
6+
[metabase.util.i18n :as i18n :refer [tru]]))
7+
8+
(use-fixtures :once (fixtures/initialize :plugins))
9+
10+
(deftest driver-deprecation-test
11+
(mt/test-driver :driver-deprecation-test-legacy
12+
(is (= :driver-deprecation-test-new
13+
(get-in (setting/properties :public) [:engines :driver-deprecation-test-legacy :superseded-by])))))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
(ns metabase.test.data.driver-deprecation-test-legacy
2+
"Dummy namespace for driver deprecation testing \"legacy\" driver, test data."
3+
(:require [metabase.test.data.sql :as sql.tx]))
4+
5+
(sql.tx/add-test-extensions! :driver-deprecation-test-legacy)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
(ns metabase.test.data.driver-deprecation-test-new
2+
"Dummy namespace for driver deprecation testing \"new\" driver, test data."
3+
(:require [metabase.test.data.sql :as sql.tx]))
4+
5+
(sql.tx/add-test-extensions! :driver-deprecation-test-new)

test/metabase/test/initialize/plugins.clj

+17-4
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,23 @@
88
[yaml.core :as yaml]))
99

1010
(defn- driver-plugin-manifest [driver]
11-
(let [manifest (io/file (format "modules/drivers/%s/resources/metabase-plugin.yaml" (name driver)))]
12-
(when (.exists manifest)
13-
(yaml/parse-string (slurp manifest)))))
11+
(let [nm (name driver)
12+
paths (mapv
13+
#(format "%s/drivers/%s/resources/metabase-plugin.yaml" % nm)
14+
;; look for driver definition in both the regular modules directory, as well as in a top-level
15+
;; test_modules directory, specifically designed for test driver definitions
16+
["modules" "test_modules"])]
17+
(first (filter some?
18+
(for [path paths
19+
:let [manifest (io/file path)]
20+
:when (.exists manifest)]
21+
(do
22+
(println (u/format-color
23+
'green
24+
"Loading plugin manifest (from %s) for driver as if it were a real plugin: %s"
25+
path
26+
nm))
27+
(yaml/parse-string (slurp manifest))))))))
1428

1529
(defn- driver-parents [driver]
1630
(let [parents-file (io/file (format "modules/drivers/%s/parents" (name driver)))]
@@ -31,7 +45,6 @@
3145
(doseq [driver drivers
3246
:let [info (driver-plugin-manifest driver)]
3347
:when info]
34-
(println (u/format-color 'green "Loading plugin manifest for driver as if it were a real plugin: %s" driver))
3548
(plugins.init/init-plugin-with-info! info)
3649
;; ok, now we need to make sure we load any depenencies for those drivers as well (!)
3750
(load-plugin-manifests! (driver-parents driver)))))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
info:
2+
name: Legacy Driver
3+
version: 0.0.0-SNAPSHOT
4+
description: Test plugin for driver deprecation (legacy)
5+
driver:
6+
name: driver-deprecation-test-legacy
7+
display-name: Driver Deprecation Test Plugin (Legacy)
8+
lazy-load: true
9+
parent: sql
10+
connection-properties:
11+
- host
12+
- port
13+
superseded-by: driver-deprecation-test-new
14+
init:
15+
- step: load-namespace
16+
namespace: metabase.driver.driver-deprecation-test-legacy

0 commit comments

Comments
 (0)