-
Notifications
You must be signed in to change notification settings - Fork 70
Chore: Fix snapshot upload filter for incompatible SQL statements #1186
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
Chore: Fix snapshot upload filter for incompatible SQL statements #1186
Conversation
| Predefined targets are available based on default Sourcegraph configurations ('docker', 'k8s'). | ||
| Custom targets configuration can be provided in YAML format with '--targets=target.yaml', e.g. | ||
| primary: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should rename this in this change, as it's unrelated to the main thing being fixed, and is also a configuration change
I also don't think this is correct? If anything it should be called frontend https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/migrations/README.md but either way, I dont think we should change that here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I suppose we call the container pgsql in some places... I'm not sure that's what we want to align on though, and either way, separate change 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted, have removed it from this PR, will submit it in a new PR. From the self-hosted customer's perspective, this database is known as the pgsql database, as it's the container / pod they deal with.
|
Thanks! but let's keep PRs one-fix-at-a-time, and also I'm not sure |
bobheadxi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
Expanded the filtering mechanism beyond commenting on extensions, to address new features in Postgres v17 which are not available in our SG Cloud pg v16 databases, as well as SQL queries / commands which break Google Cloud SQL's very fragile import.
Also renamed "primary.sql" to "pgsql.sql" to be consistent, as this is the only place across our entire codebase that this database got yet-another-name.
Test plan
Tested on customer Cloud migration
Updated unit tests
Updated process docs for IE team performing these migrations