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

Streaming deploys #249

Merged
merged 6 commits into from
Nov 29, 2023
Merged

Streaming deploys #249

merged 6 commits into from
Nov 29, 2023

Conversation

mythmon
Copy link
Member

@mythmon mythmon commented Nov 28, 2023

This is based on #187, and is going to be a bit chaotic until that merges. All my changes are in the last commit.

The idea here is that instead of deploying the existing dist directory, as done in #187, the deploy command runs its own independent build command. This will be important in the future because we plan to have that build be different than a normal dist-based build.

Then, given that the files generated for the deploy aren't all that interesting except to immediately upload them to the server, this PR now skips writing them to disk. With the exception of data loaders, files are uploaded straight from memory to the deploy server. Data loaders still write files to disk, so they can participate in caching. These use the same path as normal builds, so we can't introduce differences during deploy builds with this model.

@mythmon
Copy link
Member Author

mythmon commented Nov 28, 2023

One thing I didn't do here that I could imagine is having a sort of build pipeline, where we could build the next file while the previous file is uploading. This might become important for very large projects, but it seems to disruptive to introduce into the build process right now.

bin/observable.ts Outdated Show resolved Hide resolved
@mythmon mythmon force-pushed the mythmon/streaming-uploads branch from e343524 to c084f5e Compare November 29, 2023 17:22
@mythmon mythmon marked this pull request as ready for review November 29, 2023 17:23
@mythmon mythmon requested review from mcglincy and mbostock November 29, 2023 17:23
@mythmon
Copy link
Member Author

mythmon commented Nov 29, 2023

I rebased this on main and did some minor clean up to sync it to Matt's latest PR version. This is ready for review now.

@mythmon mythmon requested a review from cinxmo November 29, 2023 17:24
src/build.ts Outdated Show resolved Hide resolved
src/build.ts Outdated Show resolved Hide resolved
src/build.ts Outdated Show resolved Hide resolved
src/build.ts Outdated Show resolved Hide resolved
src/deploy.ts Outdated Show resolved Hide resolved
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Respecting verbose is the only blocker, but some other suggestions too. A lovely change overall!

@mythmon mythmon requested a review from mbostock November 29, 2023 19:16
@mythmon mythmon linked an issue Nov 29, 2023 that may be closed by this pull request
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@mythmon mythmon merged commit 221120b into main Nov 29, 2023
1 check passed
@mythmon mythmon deleted the mythmon/streaming-uploads branch November 29, 2023 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

observable deploy should support --root and --output
2 participants