Add DatabaseClient support #13

wiltsecarpenter · 2023-10-18T00:12:31Z

This PR adds support for the DatabaseClient API using the Observable database proxy service. This allows notebook pages to use databases that are defined and running via the observable-database-proxy command. The DatabaseClient function implemented here provides a subset of the functions found in the Observable Database Client specification,

db.query(SELECT statement, [params...]) - SQL query function style
db.sql`SELECT statement ${param}` - Tagged template literal style

Database configuration is read from the ~/.observablehq configuration file as defined by the observable-database-proxy command. The configuration includes the server address for the database proxy and a shared secret value used as an authorization header bearer token. The configuration for referenced databases is included in the generated page for preview and build of CLI projects.

Example

Source:

# Database test

```js
const db = DatabaseClient("dwh-local-proxy");
```

Database name is ${db.name}.

## query example
```js
Inputs.table(result)
```

```js
let result = await db.sql`SELECT * FROM fct_user_events LIMIT ${limit}`;
```

```js
let limit = 10;
```

## tagged template literal example

There are ${(await db.sql`SELECT COUNT(*) AS win64 FROM fct_user_events WHERE BROWSER_ID LIKE '%Win64%'`)[0].WIN64} Win64 user agent rows

Output:

Note

Using this feature requires un-submitted enhancements to the observable-notebook-proxy command.

mbostock

Neat! Thanks for taking this on. 🙏

Let’s chat about the security model for this tomorrow. In particular, I’d like to explore whether we can continue to use static analysis to tighten this down, i.e., can we prevent a notebook from being able to successfully make a request to /_database/<name>/token if that notebook doesn’t reference that DatabaseClient statically?

One way would be to transpile references to DatabaseClient so that they include a secure token, e.g., transpile DatabaseClient("foo") to DatabaseClient("foo", "<initial-token>"). That initial token could be used to make requests to /_database/<name>/token, and subsequently to /query. Imported/dynamic code wouldn’t be able to guess the initial token, and therefore wouldn’t be able to initiate a database query.

…e/database

mootari · 2023-10-23T13:02:15Z

Imported/dynamic code wouldn’t be able to guess the initial token

What would prevent code from parsing the inlined token from the document source and initiating a separate connection?

mbostock · 2023-10-23T16:13:43Z

What would prevent code from parsing the inlined token from the document source and initiating a separate connection?

Nothing, if you’re on a page that statically references a DatabaseClient. But the token wouldn’t be present in the source if the page doesn’t statically reference a DatabaseClient.

…e/database

mbostock · 2023-10-23T16:59:15Z

#46

…e/database

wiltsecarpenter · 2023-10-24T21:01:41Z

After initial discussion, I re-implemented the token passing strategy to include the tokens in the rendered HTML instead of making a separate request for them. This is implemented using a general cell "resolver" that can be enhanced to support other types of tokens, like Secrets, and to also support other methods of passing in configuration values like command line flags and environment variables.

public/client.js

cinxmo · 2023-10-24T22:46:52Z

currently yarn global add @observablehq/database-proxy doesn't work for our team because the @observablehq package points to our Github registry instead of the default NPM registry as part of the bi setup

cinxmo · 2023-10-24T23:43:29Z

public/database.js

+    this.#token = token;
+  }
+  async query(sql, params, {signal} = {}) {
+    const response = await fetch(`${this.#token.url}query`, {


is this supposed to call the /query endpoint and if so is #token.url guaranteed to end with /?

Yes, the token.url is created within makeCLIResolver in resolver.ts and will end with a /.

I think we should first normalize the url and then append "/query" to it to avoid this confusion. When I tested the current database proxy workflow, the url stored in ~/.observablehq doesn't end with a "/". I'm missing something here...

also what are the additional changes that need to be made to @observablehq/database-proxy?

Okay, changed to use URL here too. Note that the token.url has already gone through normalization in the makeCLIResolver, it is dervied from the host and port values ~/.observablehq.

mbostock · 2023-10-26T17:36:17Z

src/build.ts

+      root: sourceRoot,
+      path,
+      pages,
+      resolver: await makeCLIResolver()


I suggest lifting this up, before this for loop, so we can share it across pages rather than creating separate resolvers for each.

mbostock · 2023-10-26T17:42:59Z

src/resolver.ts

+  return `${Buffer.from(data).toString("base64") + "." + Buffer.from(hmac).toString("base64")}`;
+}
+
+export async function makeCLIResolver(): Promise<CellResolver> {


Should we call this makeCellResolver instead of makeCLIResolver?

We can name it that if we want to, but the point of "CLI" here is to differentiate it from what will be needed in the platform where we will need a "makePlatformResolver".

The “makePlatformResolver” wouldn’t live in this codebase, presumably?

mbostock

This seems very close to me. My biggest questions are:

What changes are needed in the self-hosted database proxy? We should review those concurrently.
I’d like to have a <link rel=modulepreload> for database.js if a page references a DatabaseClient, so that the page loads faster (rather than waiting until the first time a DatabaseClient is referenced). That should be pretty easy to add.

…e/database

wiltsecarpenter · 2023-10-30T15:06:59Z

See observablehq/database-proxy#85 for changes to the database-proxy that enable the CLI preview to be used with it. More changes will likely be needed.

cinxmo

LGTM with one nit. I think to have <link rel=modulepreload> for database.js, we'd need to concat the output of parseResult.databases here: https://github.com/observablehq/cli/blob/4343c4f283eace660b5716f02a22b53d6b8fa783/src/render.ts#L65-L68. Or we can add that logic to getImportMap but that seems to be for npm packages as opposed to local modules

…e/database

wiltsecarpenter · 2023-10-31T16:36:29Z

I added the module preload for database.js

mbostock · 2023-10-31T17:15:52Z

src/resolver.ts

+  url: string;
+}
+
+const configFile = join(homedir(), ".observablehq");


I think we want this config file to be scoped per-project rather than in the user’s home directory. Presumably you might want different databases per project.

mbostock · 2023-10-31T17:17:13Z

src/resolver.ts

+            url.protocol = db.ssl !== "disabled" ? "https:" : "http:";
+            url.host = db.host;
+            url.port = String(db.port);
+            url.toString();


This line should be removed.

mbostock · 2023-10-31T17:24:24Z

src/preview.ts

+  for (const item of diff) {
+    if (item.type === "add") {
+      for (const addItem of item.items) {
+        if (addItem.type === "cell" && "databases" in addItem) {


Should we remove the "databases" in addItem check here, and instead leave it to the resolver to decide whether any resolution is necessary?

Add DatabaseClient support

0efed9d

mbostock reviewed Oct 18, 2023

View reviewed changes

wiltsecarpenter force-pushed the wiltse/database branch 2 times, most recently from 439f08b to 0efed9d Compare October 18, 2023 17:02

Merge branch 'main' of https://github.com/observablehq/cli into wilts…

93764d7

…e/database

Merge branch 'main' of https://github.com/observablehq/cli into wilts…

260c9d2

…e/database

wiltsecarpenter added 5 commits October 24, 2023 10:15

Implement a render-time resolver for databases

7d6503a

Revert unneeded test changes

2a28b39

Fill out queryTag method

200df4c

Touch ups

00bc7dd

Merge branch 'main' of https://github.com/observablehq/cli into wilts…

6e3565f

…e/database

wiltsecarpenter requested a review from cinxmo October 24, 2023 21:01

mbostock reviewed Oct 24, 2023

View reviewed changes

public/client.js Outdated Show resolved Hide resolved

mbostock reviewed Oct 24, 2023

View reviewed changes

public/client.js Outdated Show resolved Hide resolved

Review feedback and error handling fix

0f40143

cinxmo reviewed Oct 24, 2023

View reviewed changes

wiltsecarpenter requested review from mbostock and cinxmo October 25, 2023 19:05

mbostock reviewed Oct 26, 2023

View reviewed changes

Merge branch 'main' of https://github.com/observablehq/cli into wilts…

25e997a

…e/database

Review feedback

4343c4f

cinxmo approved these changes Oct 30, 2023

View reviewed changes

wiltsecarpenter added 2 commits October 30, 2023 15:41

Merge branch 'main' of https://github.com/observablehq/cli into wilts…

6f28eea

…e/database

Add database.js preload

9c0c5a1

wiltsecarpenter merged commit 1bd179c into main Oct 31, 2023

wiltsecarpenter deleted the wiltse/database branch October 31, 2023 16:36

mbostock reviewed Oct 31, 2023

View reviewed changes

mbostock mentioned this pull request Oct 31, 2023

style nits #82

Closed

cinxmo mentioned this pull request Nov 3, 2023

v0.0.34 #93

Merged

net mentioned this pull request Feb 16, 2024

Fetch and cache data at runtime #839

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DatabaseClient support #13

Add DatabaseClient support #13

wiltsecarpenter commented Oct 18, 2023 •

edited

Loading

mbostock left a comment

mootari commented Oct 23, 2023

mbostock commented Oct 23, 2023

mbostock commented Oct 23, 2023

wiltsecarpenter commented Oct 24, 2023

cinxmo commented Oct 24, 2023

cinxmo Oct 24, 2023 •

edited

Loading

wiltsecarpenter Oct 25, 2023

cinxmo Oct 26, 2023

cinxmo Oct 26, 2023

wiltsecarpenter Oct 26, 2023

mbostock Oct 26, 2023

wiltsecarpenter Oct 26, 2023

mbostock Oct 26, 2023

wiltsecarpenter Oct 26, 2023

mbostock Oct 31, 2023

mbostock left a comment •

edited

Loading

wiltsecarpenter commented Oct 30, 2023

cinxmo left a comment

wiltsecarpenter commented Oct 31, 2023

mbostock Oct 31, 2023 •

edited

Loading

mbostock Oct 31, 2023

mbostock Oct 31, 2023

Add DatabaseClient support #13

Add DatabaseClient support #13

Conversation

wiltsecarpenter commented Oct 18, 2023 • edited Loading

Example

Note

mbostock left a comment

Choose a reason for hiding this comment

mootari commented Oct 23, 2023

mbostock commented Oct 23, 2023

mbostock commented Oct 23, 2023

wiltsecarpenter commented Oct 24, 2023

cinxmo commented Oct 24, 2023

cinxmo Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbostock left a comment • edited Loading

Choose a reason for hiding this comment

wiltsecarpenter commented Oct 30, 2023

cinxmo left a comment

Choose a reason for hiding this comment

wiltsecarpenter commented Oct 31, 2023

mbostock Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wiltsecarpenter commented Oct 18, 2023 •

edited

Loading

cinxmo Oct 24, 2023 •

edited

Loading

mbostock left a comment •

edited

Loading

mbostock Oct 31, 2023 •

edited

Loading