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

CI: Replace pgsanity with libpg-query #1190

Merged
merged 1 commit into from
Dec 11, 2022
Merged

Conversation

joshbwlng
Copy link
Contributor

Replacing pgsanity as it hasn't been updated in several years and is unable to properly parse JSON operators.

Change-type: patch
Signed-off-by: Josh Bowling [email protected]


libpg-query repo: https://github.com/pyramation/libpg-query-node
Issue regarding pgsanity's inability to parse JSON operators: markdrago/pgsanity#29

While libpg-query returns a full parse tree for the given SQL statement, I only real care that it throws when it fails to parse the SQL its given. This is similar to what we currently get from pgsanity, making it essentially a drop-in replacement that is more active (updated last month vs pgsanity being updated ~5 years ago) and is able to parse more recent operators etc.

Also adding an extra block to the SQL validity test to check async migration TypeScript files.

@joshbwlng joshbwlng self-assigned this Dec 9, 2022
@joshbwlng joshbwlng marked this pull request as ready for review December 9, 2022 07:24
Copy link
Contributor

@fisehara fisehara left a comment

Choose a reason for hiding this comment

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

looks real good to me.
I wonder if we want to move the migraton sanity check into pinejs and give it just the model configuration That we we may not need it in open-balena-api and balena-api

@joshbwlng
Copy link
Contributor Author

joshbwlng commented Dec 9, 2022

@fisehara I was struggling with that myself. We did something similar in AutumnDB, add testing utilities to the lowest part of the stack that made sense and then have consumers at the higher parts of the stack use the shared utility. Is that basically what you mean?

@joshbwlng joshbwlng marked this pull request as draft December 9, 2022 10:21
@fisehara
Copy link
Contributor

fisehara commented Dec 9, 2022

@joshbwlng Yes right. Have it in pinejs and let the model / config checked by pinejs. Basically, pinejs will run them, right?

@joshbwlng joshbwlng force-pushed the joshbwlng/replace-pgsanity branch from d4f3a30 to aa92051 Compare December 9, 2022 10:33
@joshbwlng
Copy link
Contributor Author

@fisehara Yeah, more or less. I'm not sure what that would look like exactly in pinejs, but I'm wondering if that could wait for a later PR? My main goal here is to just replace the outdated pgsanity, and ensure that async migration sql is checked (when possible). I agree that it might make sense to abstract shared testing utilities/methods, but I'd rather not let that block this work unless its super quick and easy to implement in pinejs.

@fisehara
Copy link
Contributor

fisehara commented Dec 9, 2022

@joshbwlng it's absolutely fine to have it on a later PR and move it down the stack into pinjs later. Just wondered, if there is anything that would block it from this current perspective.
From my side LGTM

@joshbwlng joshbwlng marked this pull request as ready for review December 10, 2022 13:35
@joshbwlng
Copy link
Contributor Author

@balena-ci rebase

@ghost ghost force-pushed the joshbwlng/replace-pgsanity branch from aa92051 to f22b95e Compare December 11, 2022 07:06
@balena-ci balena-ci enabled auto-merge December 11, 2022 07:13
Replacing pgsanity as it hasn't been updated in several years and
is unable to properly parse JSON operators.

Change-type: patch
Signed-off-by: Josh Bowling <[email protected]>
@joshbwlng joshbwlng force-pushed the joshbwlng/replace-pgsanity branch from f22b95e to 3602274 Compare December 11, 2022 07:54
@joshbwlng
Copy link
Contributor Author

@balena-ci I self certify!

@balena-ci balena-ci merged commit 3f85f94 into master Dec 11, 2022
@balena-ci balena-ci deleted the joshbwlng/replace-pgsanity branch December 11, 2022 08:10
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