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.

Sign up for GitHub

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

Jump to bottom

Require that ASTRO_STUDIO_REMOTE_DB_URL is defined at runtime #10533

Merged
merged 3 commits into from
Mar 22, 2024
Merged

Require that ASTRO_STUDIO_REMOTE_DB_URL is defined at runtime #10533

merged 3 commits into from
Mar 22, 2024

Conversation

matthewp
Copy link
Contributor

Changes

  • When we build we always get the app token from process.env.ASTRO_STUDIO_APP_TOKEN at runtime.
  • createRemoteDatabaseClient now checks that it has an app token and throws without one.

Testing

  • SSR test added in the basics fixture. Using the mock services this tests that production works, can test other production things here.
  • Added test specifically for when there is not a runtime ASTRO_STUDIO_APP_TOKEN, to check that it throws.

Docs

N/A, bug fix

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 22, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 8532164

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

martrapp
martrapp approved these changes Mar 22, 2024
View reviewed changes
Copy link
Member

@martrapp martrapp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

bholmesdev
bholmesdev reviewed Mar 22, 2024
View reviewed changes
Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

Not even remotely worried about support issues now 👍

Left a question on simplifying db execute.

packages/db/src/core/cli/commands/execute/index.ts Show resolved Hide resolved
@matthewp matthewp merged commit 6576f5d into main Mar 22, 2024
14 checks passed
@matthewp matthewp deleted the app-token-required branch March 22, 2024 15:52
@astrobot-houston astrobot-houston mentioned this pull request Mar 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bholmesdev bholmesdev bholmesdev approved these changes

@martrapp martrapp martrapp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matthewp @bholmesdev @martrapp