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

build: add deployment previews for vega-lite documentation site #9277

Closed
wants to merge 2 commits into from

Conversation

hydrosquall
Copy link
Member

@hydrosquall hydrosquall commented Mar 3, 2024

Motivation

Changes

Try configuring https://github.com/rossjrw/pr-preview-action

Testing

  • The comment on this PR should link to a version of the docs site that outputs a console.log every time the vega-lite compile function is called.

Notes

@hydrosquall hydrosquall requested a review from a team as a code owner March 3, 2024 14:47
Copy link

github-actions bot commented Mar 3, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://vega.github.io/vega-lite-pr-previews/pr-preview/pr-9277/
on branch gh-pages at 2024-03-03 16:11 UTC

@hydrosquall hydrosquall force-pushed the cameron.yick/deployment-preview-spike branch from e9be4c4 to dea91e0 Compare March 3, 2024 14:52
@hydrosquall hydrosquall marked this pull request as draft March 3, 2024 14:53
@hydrosquall hydrosquall force-pushed the cameron.yick/deployment-preview-spike branch 3 times, most recently from eede4e1 to 6c6a000 Compare March 3, 2024 15:05
@hydrosquall
Copy link
Member Author

hydrosquall commented Mar 3, 2024

It looks like this action doesn't work The site is built ( https://github.com/vega/vega-lite/tree/gh-pages/pr-preview/pr-9277 ), but the address doesn't work because the repository is not set up to serve from a branch. It looks like you can either have (Github Actions) or (Github Pages) set up, but not both.

image

I'm going to leave this alone until we have a chance to discuss, as we can avoid needing to configure a custom action if we're willing to use a 3rd party provider (StackBlitz, cloudflare pages, vercel, netlify, etc) to handle this for us. Alternately, we could let the previews be published to a separate repository.

@hydrosquall hydrosquall force-pushed the cameron.yick/deployment-preview-spike branch 8 times, most recently from d0730bb to 05a0d27 Compare March 3, 2024 15:58
@hydrosquall hydrosquall force-pushed the cameron.yick/deployment-preview-spike branch from 05a0d27 to 78c456b Compare March 3, 2024 16:09
@hydrosquall
Copy link
Member Author

After looking at the size of the pushes to gh-pages, I don't think deploying the preview site based on GH pages is a good idea, even if it were possible to do so inside this repo. Each new PR would generate a diff millions of lines long due to all the compiled JSON examples. Unless our maintainers and contributors remember to not download the gh-pages branch, this is a lot of unnecessary bloat in the github repository. I think this is part of why Github is trying to deprecate the branch-based deployment model, and keep the deployment artifacts outside of Git.

I'm inclined to recommend that we do this without native github actions, and instead choose one of the providers that offer deployment preview for free to open source projects. Let me know what you think @domoritz , and we can go from there.

@nicolaskruchten
Copy link

The way I've solved this in the past is not to commit to gh-pages as a child commit of the existing one, but rather as a whole new parent-less commit. This way there are no diffs, just a bunch of orphaned commits that eventually go away or at least don't really bother anyone.

@hydrosquall hydrosquall linked an issue Mar 3, 2024 that may be closed by this pull request
1 task
@hydrosquall
Copy link
Member Author

@nicolaskruchten thanks for taking a look. Do you have a link to a public repository where you had set this up?

I'm not sure that the GH pages route for branch deploys will work inside this repo, but maybe what you're describing is something we could try in a repo dedicated to just hosting PR previews, e.g. https://github.com/vega/vega-lite-pr-previews

@nicolaskruchten
Copy link

Here you go https://github.com/plotly/plotly.py/blob/master/.circleci/config.yml#L558-L567 ... I was surprised that this worked but it does! The target for this is here https://github.com/plotly/plotly.py-docs/tree/gh-pages (which renders at https://plotly.com/python-api-reference/) and it's always a single commit with no parent. Presumably the other commits are piling up somewhere in Github's infra and getting garbage-collected? I'm not sure but no one has ever complained to my knowledge :)

@nicolaskruchten
Copy link

To get deployment previews per-PR is probably a lot trickier than this, though. Incredibly valuable but trickier :) We've done this in the past with Heroku's non-free features, and maybe there's a workflow with a provider with hobby plans? Fly.io doesn't collect on monthly bills under $5 so maybe that's an option since these deployment previews will get very little traffic?

@domoritz
Copy link
Member

Thanks for making a POC for this. I think hosting previews in a separate repo sounds great. Alternatively, I've seen vercel/netflify deploy PRs and that could be an alternative. That's what https://github.com/vega/react-vega does for example (@kristw set that up). I don't care either way as long as we don't pollute the main repo.

- reopened
- synchronize
- closed
# TODO: should this be rerun on new commits?
Copy link
Member

Choose a reason for hiding this comment

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

Yes

concurrency: preview-${{ github.ref }}

jobs:
# TODO: do we still want to run these on dependabot/non-human branches?
Copy link
Member

Choose a reason for hiding this comment

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

No. That would be too much noise. I've never had to check dependabot PRs.

# TODO: do we still want to run these on dependabot/non-human branches?
deploy-preview-docs:
if: github.event.action != 'closed'
runs-on: ubuntu-20.04
Copy link
Member

Choose a reason for hiding this comment

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

Why not latest?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to avoid surprise breakages by pinning to a specific version.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not worried about os updates.

@hydrosquall
Copy link
Member Author

Replaced by #9294

@hydrosquall hydrosquall closed this Apr 7, 2024
@hydrosquall hydrosquall deleted the cameron.yick/deployment-preview-spike branch April 7, 2024 15:27
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.

developer-experience: deployment previews for pull requests
3 participants