feat: add postgres support + migrations

[dr-carrot]

Description

Screenshot (if UI-related)

To-Dos

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

Issues Fixed or Closed

• Fixes [Feature Request] Add support for external database connections #186

TODO:

• An extra migration in the develop branch
• Translation keys?
• Docs
• Test image - drcarrot/jellyseerr-postgres:latest

[Fallenbagel]

Whats the difference between this and #421?

[dr-carrot]

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

[dr-carrot]

Looks good on my end, but can we get /app/config/settings.json info stored as well? Seems like it should be in the database.

I think migrating the config file to the db is worth its own issue and PR. After this is merged

[ralgar]

Looks good on my end, but can we get /app/config/settings.json info stored as well? Seems like it should be in the database.

Not sure I agree with this idea. IMO config should be in files and only data should be in the DB, it's simpler and more transparent that way. What benefit do you envision from moving the configuration into the DB?

From my end, all I see is that it will break (and seriously complicate) the declarative configuration workflow that I and @aleksasiriski are both currently using:

config is mounted in an init container and copied over to jellyseerr so it's completely stateless and declarative

Jellyseerr is the easiest app to configure in my cluster, precisely because of this JSON file. Just copy it into an emptyDir and run envsubst on it, and you're good to go. Editing the ConfigMap only takes a few seconds. With *arr apps I have to dig into the DB or source code every time I want to change something, it's not much fun.

[onedr0p]

Not sure I agree with this idea. IMO config should be in files and only data should be in the DB, it's simpler and more transparent that way. What benefit do you envision from moving the configuration into the DB?

I agree with what you're saying to a point however there's a bunch of configuration in this config file that could be offloaded to the database. Obviously things like database, port and some other config should live in env and/or a config file but when it comes to a app that implements sqlite or pg most configuration should live in the database. Otherwise you start mixing configuration between env, file and database and that's not great.

config is mounted in an init container and copied over to jellyseerr so it's completely stateless and declarative

This is pretty janky however I do it sometimes as well across other apps but there's no guarantee that this config file won't change in the future which will end up breaking this pattern. For example sabnzbd is all one configuration file and the app can run migrations on the config file. If you take a snapshot of this config, do the init container dance with envsubst, and then sabnzbd releases an update that migrates the config to a version your config that you snapshotted is going to be out of date and will potentially break the app.

Another issue you run into is that you essentially make it so changing configuration in the UI is pointless because those values live in your initcontainer/configmap/envsubst solution. Any time you want to make a configuration change you must restart the application. Some values aren't even easy to guess what to put in the config file because they are calculated on save in the UI.

From my end, all I see is that it will break (and seriously complicate) the declarative configuration workflow that I and @aleksasiriski are both currently using

If the config is in the DB and you're already backing up the database there's no reason to do this. It's not like your deploying new instances of jellyseerr every hour and need declarative configuration for this app. Once it's set up that configuration lives in the database and should be backed up.

Don't get me wrong, I love applications that have full declarative configuration. I don't think jellyseerr really benefits from it due to the reasons I laid out.

[douglasparker]

I don't see a point in offloading configuration to the database. It just makes it harder to make changes.

Config files can be live reloaded without a reboot and there isn't a performance benefit to adding it to the database.

Edit: And there isn't an issue exposing to the UI either. It can be mapped to the config file as well.

I don't know, I just don't see a real good reason for the added complexity.

[onedr0p]

I don't see a point in offloading configuration to the database. It just makes it harder to make changes.

The amount of times I've to go into the Sonarr or Radarr or any other apps database to change config is pretty much zero. Most expose enough options in the config file to make sure the app starts, so you can change config in the UI.

Basically anything that requires a app restart should be in env or config file, anything that can change during run time should be in the database (since there is a database implemented in the app)

I don't know, I just don't see a real good reason for the added complexity.

The main problem I'm trying to solve is close to what @aleksasiriski and @ralgar is doing but I don't want to deal with the nuances they are faced with in their solution of templating the config file, which is possibly dealing with upstream config changes and not having access to change settings in the UI and have those reflected in the template.

But I get it, why change it if things work now for the majority of people 🤷‍♂️

[aleksasiriski]

Let's move this to a discussion so anyone interested can talk about it there :D I'll join with my perspective as well.

#651

[dr-carrot]

@Fallenbagel id appreciate a review when you (or anyone else) gets the chance. I haven’t seen or heard about any issues for a while and I’d prefer to get this merged before more migrations are required

[Fallenbagel]

@Fallenbagel id appreciate a review when you (or anyone else) gets the chance. I haven’t seen or heard about any issues for a while and I’d prefer to get this merged before more migrations are required

There is one issue. Not sure if it's isolated to this pr (but I have not been able to reproduce it on sqlite) #652

Also ill review when I can

[dr-carrot]

There is one issue. Not sure if it's isolated to this pr (but I have not been able to reproduce it on sqlite) #652

I had a look and I don’t think it’s caused by this pr directly. I think it’s either caused by a user being renamed or a bad migration. I’m going to write up an SQLite to Postgres migration guide when I have a little time

[douglasparker]

There is one issue. Not sure if it's isolated to this pr (but I have not been able to reproduce it on sqlite) #652

I had a look and I don’t think it’s caused by this pr directly. I think it’s either caused by a user being renamed or a bad migration. I’m going to write up an SQLite to Postgres migration guide when I have a little time

Yep, I'm pretty sure it's because of old tags being in place from when I used Overseerr.

I had assumed that it would see that ID: Username didn't exist, so it would make a new tag- and then I could retag stuff made from Overseerr.

It turns out that it simply maps based on ID and not the whole tag.

I've gone ahead and wiped out my tags (Not worth the trouble to fix considering some of my users still don't have an ID yet).

Issue #652 should no longer be an issue.

Extremely excited to get this merged into Jellyseerr!

Thank you so much @dr-carrot and @Fallenbagel!

[danshilm]

Can this be a timestamp without time zone to match how the datetime is stored in sqlite? I haven't tested that, so am not sure what the behaviour would be.

Edit: looks like a fork is doing just that.

[dr-carrot]

I'll give it a try and make sure it works first

[dr-carrot]

Sqlite doesn't support that. In fact, that fork dropped sqlite completely

[dr-carrot]

From the readme:

Uses PostgreSQL over SQLite (SQLite database functionality is completely removed!)

[danshilm]

My question was about Postgres actually. not using a different data type for Sqlite.
More specifically, whether this can be done without any issues since imo it's always a good idea to store timestamps without the time zone:

Suggested change

	datetime: 'timestamp with time zone',
	datetime: 'timestamp without time zone',

[danshilm]

Looking at having to have separate migrations for sqlite and postgres isn't awesome, but seems like it's the way to do it.
Did someone try adding a bogus field to an entity and seeing if generating migrations for that is easy enough? (just to test how things are)

Something that I think would be nice is to update the dev setup to include everything that's needed to work with Jellyseer whether it's with the sqlite or postgres database. That would include:

• including a postgres service in the docker compose
• setting up a .env.example file for devs to know what their .env file should look like
  • maybe use cross-env so Windows users can also easily contribute? (the .env should automatically be loaded by nextjs on unix based OSes, but I haven't tested that)
• update the migration scripts in package json to generate migrations for both sqlite and postgres

[danshilm]

There's also a fix I saw a fork have to make that I'm curious to know if someone using this pull's docker image had to do?
The commit in question

The issue seems to be TypeORM not loading the MediaRequest relations on a Media entity, and therefore dumping all the requests for a Media whenever it's updated and saved.

[dr-carrot]

There's also a fix I saw a fork have to make that I'm curious to know if someone using this pull's docker image had to do? The commit in question

The issue seems to be TypeORM not loading the MediaRequest relations on a Media entity, and therefore dumping all the requests for a Media whenever it's updated and saved.

There was something similar in this PR, but it didn't always work quite right. After some experimenting I found that using more strict constraints makes typeorm behave itself in that instance. I haven't heard of or had any issues with it since

[captmicr0]

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.