Add "observable deploy" command. #187

mcglincy · 2023-11-16T01:46:05Z

Description

Add a new "yarn deploy" command that talks to our new API endpoints.

Resolves https://github.com/observablehq/observablehq/issues/15128 and https://github.com/observablehq/observablehq/issues/15134

Internal workflow:

Requires api key to be found in .observablehq dot-file.
Creates a project if one doesn't exist. The user will be prompted for a slug and their choice of workspace (if they have more than one workspace). The new project id is then stashed in a .project file.
Creates a new deploy.
Recurses the ./dist directory, uploading each file within it.
Marks the deploy as "uploaded".

Review notes

To test locally, you'll need the feature flag enabled in your local db:

INSERT INTO user_features (feature, canary) VALUES ('cli_project', true);

Then you can login from the CLI:
NODE_TLS_REJECT_UNAUTHORIZED=0 OBSERVABLEHQ_HOST=https://observable.test:5000/ yarn tsx bin/observable.ts login

And then do a deploy from the CLI:
NODE_TLS_REJECT_UNAUTHORIZED=0 OBSERVABLEHQ_HOST=https://observable.test:5000/ yarn deploy

Tests

Adds to ObservableApiMock introduced in #39.

Risk

Low.

bin/observable.ts

src/deploy.ts

src/toolConfig.ts

src/apiClient.ts

mcglincy · 2023-11-21T16:33:37Z

Now that Michael's auth PR is merged and we've chosen our testing strategy (undici mocking), I'll get this PR properly integrated with his code and ready for non-draft review.

mcglincy · 2023-11-21T21:28:57Z

PR updated and now ready for actual review. Thanks all!

src/deploy.ts

src/auth.ts

src/deploy.ts

src/toolConfig.ts

test/deploy-test.ts

test/mocks/observableApi.ts

cinxmo

some concerns around error handling - will review the test cases later! (I also needed the OBSERVABLEHQ_API_HOST env var for testing locally 😆)

src/observableApiClient.ts

src/toolConfig.ts

cinxmo

some minor suggestions but it worked well from testing locally! will approve after the small changes and increasing test coverage

src/auth.ts

…ing postDeployFile test breakage.

Co-authored-by: Michael Cooper <[email protected]>

mbostock · 2023-11-29T02:50:26Z

src/deploy.ts

+
+  // Mark the deploy as uploaded.
+  await apiClient.postDeployUploaded(deployId);
+  logger.log(`Deployed project now visible at ${getObservableUiHost()}/p/${projectId}`);


Is the projectId the hexadecimal identifier? We should try to never show that to the user, especially for the URL that they are likely to copy-paste and share.

We might also have the API return the URL for the project, so that we can change the logic more easily on the API side without needing clients to update.

@mythmon also raised the possibility of replacing projectId in any visible URLs with workspace + slug.

We're about to hammer out the PR for project serving on the server side, so can finalize the visible URL format there, and then fix the CLI deploy messaging accordingly. #270 on me for that.

mbostock · 2023-11-29T02:53:00Z

src/observableApiClient.ts

+      headers: [...this._apiHeaders, ["Content-Type", "application/json"]],
+      body: JSON.stringify({})
+    });
+    if (response.status !== 200) {


Use response.ok here? (And below.)

mbostock · 2023-11-29T02:57:11Z

src/toolConfig.ts

-
-interface Config {
+const observableConfigName = ".observablehq";
+interface ObservableConfig {


“ObservableConfig” sounds a little ambiguous. Should we call this “UserConfig” (since it typically lives in the user’s home directory)?

mbostock

Nice job with the tests! 👏

mcglincy · 2023-11-29T03:40:12Z

Will address comments in a followup PR. Thanks!

Fil reviewed Nov 16, 2023

View reviewed changes

bin/observable.ts Outdated Show resolved Hide resolved

Fil reviewed Nov 16, 2023

View reviewed changes

src/deploy.ts Outdated Show resolved Hide resolved

Fil reviewed Nov 16, 2023

View reviewed changes

src/deploy.ts Outdated Show resolved Hide resolved

Fil reviewed Nov 16, 2023

View reviewed changes

src/toolConfig.ts Outdated Show resolved Hide resolved

mcglincy changed the title ~~Add deploy command.~~ Add "observable deploy" command. Nov 17, 2023

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/apiClient.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/apiClient.ts Outdated Show resolved Hide resolved

Add observable deploy command.

f656fd7

mcglincy force-pushed the mcglincy/observable-deploy branch from 92bee89 to f656fd7 Compare November 21, 2023 21:12

mcglincy marked this pull request as ready for review November 21, 2023 21:14

Merge main and resolve conflicts.

bc19544

mcglincy requested review from mythmon, Fil and cinxmo November 21, 2023 21:23

mcglincy added 2 commits November 21, 2023 14:25

De-lint.

3fc6cd9

Keep yarn.lock from main.

d2642ce

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/deploy.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/auth.ts Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/auth.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/deploy.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/deploy.ts Show resolved Hide resolved

cinxmo reviewed Nov 21, 2023

View reviewed changes

src/deploy.ts Outdated Show resolved Hide resolved

mythmon requested changes Nov 21, 2023

View reviewed changes

cinxmo suggested changes Nov 21, 2023

View reviewed changes

mcglincy added 4 commits November 22, 2023 08:06

Address review comments.

9ff7870

De-lint.

3533bc4

Wordsmith auth message.

6bdb021

Merge main and resolve conflicts.

f299b83

mcglincy added 2 commits November 27, 2023 13:24

Remove trailing whitespace.

858a939

Switch to visitFiles.

1f12cb7

cinxmo reviewed Nov 27, 2023

View reviewed changes

src/observableApiClient.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 27, 2023

View reviewed changes

src/toolConfig.ts Outdated Show resolved Hide resolved

cinxmo reviewed Nov 27, 2023

View reviewed changes

mythmon mentioned this pull request Nov 28, 2023

Streaming deploys #249

Merged

mcglincy added 2 commits November 28, 2023 08:15

Fix whitespace.

4ac9f45

Refactor and add tests to get to 100% test coverage.

7831c4d

mcglincy requested a review from cinxmo November 28, 2023 16:37

cinxmo approved these changes Nov 28, 2023

View reviewed changes

mcglincy added 4 commits November 28, 2023 10:51

Test cleanup.

9fa3ae1

Remove debug method.

246c695

Add test cases for all possibly-throwing API calls.

58c0d71

Merge branch 'main' into mcglincy/observable-deploy

4325b2d

mythmon approved these changes Nov 28, 2023

View reviewed changes

mythmon reviewed Nov 28, 2023

View reviewed changes

src/auth.ts Outdated Show resolved Hide resolved

mcglincy and others added 6 commits November 28, 2023 16:23

Change to deploy.config written in ./docs/.observablehq. Still debugg…

d433749

…ing postDeployFile test breakage.

Update src/auth.ts

b93bc17

Co-authored-by: Michael Cooper <[email protected]>

Add missing new deploy parameter.

f126a45

Merge branch 'main' into mcglincy/observable-deploy

9c3a536

Change deploy params to sourceRoot and deployRoot.

54e58ac

Reorder, rename.

7a5d05e

mcglincy merged commit 8cb5afb into main Nov 29, 2023
1 check passed

mcglincy deleted the mcglincy/observable-deploy branch November 29, 2023 02:31

mbostock reviewed Nov 29, 2023

View reviewed changes

mcglincy mentioned this pull request Nov 29, 2023

Address post-merge deploy PR comments. #271

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "observable deploy" command. #187

Add "observable deploy" command. #187

mcglincy commented Nov 16, 2023 •

edited

Loading

mcglincy commented Nov 21, 2023

mcglincy commented Nov 21, 2023

cinxmo left a comment •

edited

Loading

cinxmo left a comment

mbostock Nov 29, 2023

mcglincy Nov 29, 2023 •

edited

Loading

mbostock Nov 29, 2023 •

edited

Loading

mbostock Nov 29, 2023

mbostock left a comment

mcglincy commented Nov 29, 2023

Add "observable deploy" command. #187

Add "observable deploy" command. #187

Conversation

mcglincy commented Nov 16, 2023 • edited Loading

Description

Review notes

Tests

Risk

mcglincy commented Nov 21, 2023

mcglincy commented Nov 21, 2023

cinxmo left a comment • edited Loading

Choose a reason for hiding this comment

cinxmo left a comment

Choose a reason for hiding this comment

mbostock Nov 29, 2023

Choose a reason for hiding this comment

mcglincy Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

mbostock Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

mbostock Nov 29, 2023

Choose a reason for hiding this comment

mbostock left a comment

Choose a reason for hiding this comment

mcglincy commented Nov 29, 2023

mcglincy commented Nov 16, 2023 •

edited

Loading

cinxmo left a comment •

edited

Loading

mcglincy Nov 29, 2023 •

edited

Loading

mbostock Nov 29, 2023 •

edited

Loading