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

Fail the shell fast if you cannot query the database #509

Merged
merged 6 commits into from
Dec 11, 2024
Merged

Conversation

ecooper
Copy link

@ecooper ecooper commented Dec 11, 2024

Problem

If you have an invalid database or something wacky with your secret, you won't know until you run the first command in your shell.

Solution

Instead, do a quick queryable check and throw a validation error if the db is unqueryable for whatever reason. This will also exercise creds checking so that will fail fast too.

Additionally, 404s from frontdoor are thrown as ValidationError now since that happens outside the scope of isQueryable.

Result

Shell will fail immediately if you cannot query the database or with the secret you've provided.

Testing

isQueryable is injected into the container, and we have a unit test to prove things fail.

Copy link
Contributor

@ptpaterson ptpaterson left a comment

Choose a reason for hiding this comment

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

Yes, please, and thank you!

isQueryable sounds like it ought to return a boolean, but that's a pretty pedantic nit.

export const isQueryable = async (argv) => {
const runQueryFromString = container.resolve("runQueryFromString");
try {
await runQueryFromString("1+1", argv);
Copy link
Contributor

Choose a reason for hiding this comment

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

big ❤️

@ecooper ecooper merged commit 02b5166 into v3 Dec 11, 2024
4 checks passed
@ecooper ecooper deleted the v3-fast-fail-shell branch December 11, 2024 22:03
@cleve-fauna cleve-fauna mentioned this pull request Dec 13, 2024
@wildemat wildemat mentioned this pull request Dec 18, 2024
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.

3 participants