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

feat: add postgres support + migrations #628

Open
wants to merge 59 commits into
base: develop
Choose a base branch
from

Conversation

dr-carrot
Copy link

@dr-carrot dr-carrot commented Jan 19, 2024

Description

Screenshot (if UI-related)

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract
  • Database migration (if required)

Issues Fixed or Closed

TODO:

  • An extra migration in the develop branch
  • Translation keys?
  • Docs
  • Test image - drcarrot/jellyseerr-postgres:latest
  • Move new docs from readme to the newer docs page
  • Switch date column types to remove timezone

@dr-carrot dr-carrot changed the title V1.7.0/postgresql feat: add postgres support + migrations Jan 19, 2024
@Fallenbagel
Copy link
Owner

Whats the difference between this and #421?

@dr-carrot
Copy link
Author

Whats the difference between this and #421?

This PR contains all the changes in #421 plus changes from @ralgar that are also mentioned in that PR. Additionally, this PR contains the migration script required to get postgres running, documentation on how to configure postgres, and improved ssl configuration options

@captmicr0
Copy link

I've been using this branch for a couple months and semi-regularly get these two errors in my Postgres logs.
I did migrate from an existing sqlite database so I'm guessing this error is the result of that.

[5593] ERROR:  null value in column "mediaId" of relation "media_request" violates not-null constraint

[5593] DETAIL:  Failing row contains (541, 2, 2024-06-05 00:53:25.891558-04, 2024-06-05 00:53:25.933927-04, movie, null, 3, 3, f, null, null, null, null, null, f).

[5593] STATEMENT:  UPDATE "media_request" SET "mediaId" = $1, "updatedAt" = CURRENT_TIMESTAMP WHERE "id" = $2
[5593] ERROR:  syntax error at or near ")" at character 1610

[5593] STATEMENT:  SELECT "media"."id" AS "media_id", "media"."mediaType" AS "media_mediaType", "media"."tmdbId" AS "media_tmdbId", "media"."tvdbId" AS "media_tvdbId", "media"."imdbId" AS "media_imdbId", "media"."status" AS "media_status", "media"."status4k" AS "media_status4k", "media"."createdAt" AS "media_createdAt", "media"."updatedAt" AS "media_updatedAt", "media"."lastSeasonChange" AS "media_lastSeasonChange", "media"."mediaAddedAt" AS "media_mediaAddedAt", "media"."serviceId" AS "media_serviceId", "media"."serviceId4k" AS "media_serviceId4k", "media"."externalServiceId" AS "media_externalServiceId", "media"."externalServiceId4k" AS "media_externalServiceId4k", "media"."externalServiceSlug" AS "media_externalServiceSlug", "media"."externalServiceSlug4k" AS "media_externalServiceSlug4k", "media"."ratingKey" AS "media_ratingKey", "media"."ratingKey4k" AS "media_ratingKey4k", "media"."jellyfinMediaId" AS "media_jellyfinMediaId", "media"."jellyfinMediaId4k" AS "media_jellyfinMediaId4k", "watchlist"."id" AS "watchlist_id", "watchlist"."ratingKey" AS "watchlist_ratingKey", "watchlist"."mediaType" AS "watchlist_mediaType", "watchlist"."title" AS "watchlist_title", "watchlist"."tmdbId" AS "watchlist_tmdbId", "watchlist"."createdAt" AS "watchlist_createdAt", "watchlist"."updatedAt" AS "watchlist_updatedAt", "watchlist"."requestedById" AS "watchlist_requestedById", "watchlist"."mediaId" AS "watchlist_mediaId" FROM "media" "media" LEFT JOIN "watchlist" "watchlist" ON "watchlist"."mediaId"="media"."id" AND ("media"."id"= "watchlist"."mediaId" and "watchlist"."requestedById" = $1) WHERE "media"."tmdbId" in ()

This one seems to be because there is no value provided for the WHERE "media"."tmdbId" in () part of the statement.

@github-actions github-actions bot added the merge conflict Cannot merge due to merge conflicts label Jun 13, 2024
Copy link

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@Lebowski89
Copy link

Lebowski89 commented Jun 20, 2024

Keen to get this merged. Has been working great for me. Jumped over from overseerr just for this postgres support. Edit: Also, hope this isn't being held up because of any potential issues with migrating from sqlite to postgres. It should be the radarr/lidarr/sonarr/whisparr (etc) approach, where they officially support new instances with postgres and only offer some suggestions for migrating to postgres with no official support (see: https://wiki.servarr.com/radarr/postgres-setup).

Migrating an existing sqlite3 database is unsupported, and this script may not work without modifications which we cannot assist you with. We support only new installs using postgres.

@croneter
Copy link

Same. What's blocking a merge appart from an obvious rebase..?

@github-actions github-actions bot removed the merge conflict Cannot merge due to merge conflicts label Jun 21, 2024
package.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Remove this file from this pr

import { Column } from 'typeorm';

const pgTypeMapping: { [key: string]: ColumnType } = {
datetime: 'timestamp with time zone',
Copy link
Owner

Choose a reason for hiding this comment

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

I believe it's best practice to always store time in db without the timezones

@Fallenbagel
Copy link
Owner

Same. What's blocking a merge appart from an obvious rebase..?

I added some review comments long ago. Forgot to submit it lol.

Also our focus for now is to get #815 in

Copy link

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@github-actions github-actions bot added the merge conflict Cannot merge due to merge conflicts label Jun 23, 2024
@nodadyoushutup
Copy link

I know that this is not the current priority to get in, but do we have even a hand wave idea of potentially how long it may be? I can continue to use the preview container but I am eagerly awaiting this not being a preview feature :)

@joaopedrocg27
Copy link

Is there a timeframe to merge this? Since running in kubernetes this is a deal breaker for now

@RyuunosukeDS3
Copy link

Chiming in to say, I migrated from Overseerr to Jellyseerr for this. I'm also running in Kubernetes, and I'm trying to move everything I can to postgres.

@juanlurie
Copy link

I’d like to add this to my docker stack as well. Quite keen to have it working with Postgres

@gauthier-th gauthier-th mentioned this pull request Aug 12, 2024
9 tasks
@github-actions github-actions bot removed the merge conflict Cannot merge due to merge conflicts label Aug 19, 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.

[Feature Request] Add support for external database connections