feat(server): add postgres major version check #6213
Conversation
…upported versions The pgvecto.rs extension, though not distributed, can be built for PostgreSQL 12 and 13. An installation of PostgreSQL 12 with the pgvecto.rs extensions installed will not be caught by immich. This causes immich to attempt to run the database migrations without having a proper environment. With assertPostgresql the server will throw an error if the PostgreSQL version is not within the supported range.
|
Hey, thanks for the PR, I think I would prefer if we did this as a greater than comparison rather than an explicit list, would you be able to make that change? 😊 |
As requested by @zackpollard
If we define one we might as well use it. makes changing the versioning later easier
|
I have replaced the list with the comparison. |
|
Yea, my reasoning is basically that we don't actually officially support any version other than 14 now anyway. So limiting the versions almost feels like we have tested with versions over 14 which we don't. Running anything other than 14 is at your own risk basically, but I agree we shouldn't let people run versions under 14 as it's likely to use features that don't yet exist in those versions. |
|
You have a failing test but once that is fixed I'll get this merged in 😊 |
|
The problem is that in the tests, we use mockResolvedValueOnce. |
Works for me |
|
Would maybe be good also to add a test specifically testing this functionality, so a test that asserts the correct error is thrown when postgres version is less than 14 😄 |
`should return if minimum supported PostgreSQL and vectors version are installed`:
Check if init returns properly and that getPostgresVersion is called twice
`should thrown an error if PostgreSQL version is below minimum supported version`:
Checks if the init function correctly returns an error
`should suggest image with postgres ${major} if database is ${major}`:
Modified to set MockResolvedValue instead of MockResolvedValueOnce. With the new check we get the PostgreSQL version twice. So it needs to be set during the entire test.
`should not suggest image if postgres version is not in 14, 15 or 16`:
Modified the bounds to [14, 18]. Because values below 14 now will not get called.
Also Modified to call `getPostgresVersion.MockResolvedValueOnce` for twice, because it gets called twice.
`should thrown an error if PostgreSQL version is below minimum supported version`: The regex function I wrote mistakingly used the negate function which check that the error *did not* contain the phrase "PostgreSQL". Which is the opposite `should not suggest image if postgres version is not in 14, 15 or 16`: confused bounds for a normal javascript array. Changed the test to only check for values above 16. As values below 14 will get thrown out by test `should return if minimum supported PostgreSQL and vectors version are installed` I apologise for the mistakes in my previous commit.
|
I haven't merged this yet as I wanted to double check the tests on my PC, won't be able to do that until this evening |
jrasm91
changed the title
|
I checked with 13 and it threw and error. Then I check with 14 and it worked as expected. |
* feat(server): Throw error when PostgreSQL version is not within the supported versions The pgvecto.rs extension, though not distributed, can be built for PostgreSQL 12 and 13. An installation of PostgreSQL 12 with the pgvecto.rs extensions installed will not be caught by immich. This causes immich to attempt to run the database migrations without having a proper environment. With assertPostgresql the server will throw an error if the PostgreSQL version is not within the supported range. * Replaced assertion with lesser than comparison As requested by @zackpollard * Changed the comparison to use the minPostgresVersion variable. If we define one we might as well use it. makes changing the versioning later easier * Added two new tests, modified two existing tests `should return if minimum supported PostgreSQL and vectors version are installed`: Check if init returns properly and that getPostgresVersion is called twice `should thrown an error if PostgreSQL version is below minimum supported version`: Checks if the init function correctly returns an error `should suggest image with postgres ${major} if database is ${major}`: Modified to set MockResolvedValue instead of MockResolvedValueOnce. With the new check we get the PostgreSQL version twice. So it needs to be set during the entire test. `should not suggest image if postgres version is not in 14, 15 or 16`: Modified the bounds to [14, 18]. Because values below 14 now will not get called. Also Modified to call `getPostgresVersion.MockResolvedValueOnce` for twice, because it gets called twice. * Fixed two mistakes in the jest functions from previous commit #2abcb60 `should thrown an error if PostgreSQL version is below minimum supported version`: The regex function I wrote mistakingly used the negate function which check that the error *did not* contain the phrase "PostgreSQL". Which is the opposite `should not suggest image if postgres version is not in 14, 15 or 16`: confused bounds for a normal javascript array. Changed the test to only check for values above 16. As values below 14 will get thrown out by test `should return if minimum supported PostgreSQL and vectors version are installed` I apologise for the mistakes in my previous commit. * Format fix --------- Co-authored-by: max <wak@vanling.net>
The pgvecto.rs extension, though not distributed, can be built for PostgreSQL 12 and 13.
An installation of PostgreSQL 12 with the pgvecto.rs extensions installed will not be caught by immich.
This causes immich to attempt to run the database migrations without having a proper environment.
With assertPostgresql the server will throw an error if the PostgreSQL version is not within the supported range.