Simplify the environment variables

[purple-emily]

Theres still a lot of duplicated variables, these need merging and clarifying.

[iPromKnight]

There is even more too 👅

For the consumer, after i converted to modules, I moved them all here as modules are instantiated once
https://github.com/Gabisonfire/torrentio-scraper-sh/blob/master/src/node/consumer/src/lib/config.js

For the producer, anything in the json files in the Configuration folders can be an env var.

For jackettio, they are here: https://github.com/Gabisonfire/torrentio-scraper-sh/blob/master/src/node/addon-jackett/src/lib/settings.js

Selfhostio doesn't have them like this yet - trival to produce though

[purple-emily]

@iPromKnight yeah, I am not well versed in JS so it’s a nightmare. I’m almost learning as I’m going lol. Feel free to pull the branch from my fork and push some changes and PR it 😂

I’m done for today. It’s sit on the sofa and watch crappy TV time

[purple-emily]

@iPromKnight or @Gabisonfire

Can one of you offer me the way to change:

ScrapeConfiguration__StorageConnectionString=host=postgres;username=postgres;password=postgres;database=selfhostio;

so it will use the

# PostgreSQL
POSTGRES_HOST=postgres
POSTGRES_PORT=5432
POSTGRES_USER=postgres
POSTGRES_PASSWORD=postgres
POSTGRES_DB=selfhostio

variables instead. Ideally I want to strip out all double coded values.

I can see in this file:

https://github.com/Gabisonfire/torrentio-scraper-sh/blob/57f4757541eceda82c553e2a7ca7e7063ffb7ee8/src/producer/Extensions/ServiceCollectionExtensions.cs

and then your comment @iPromKnight:
For the producer, anything in the json files in the Configuration folders can be an env var.

[purple-emily]

I'm going to add comments into the code, if we can make a habit of this going forward I'd be a lot happier.

I can see a few undocumented environment variables, like:

const NO_CACHE = process.env.NO_CACHE || false;

I'm curious what the consequences would be if we disabled the cache, is this a functionality we wish to keep or am I safe in assuming this could be removed.

Can I confirm @iPromKnight that you have no plans to drop a 32k commit completely rewriting the addon or consumer 😂😂😂😂😂😂

If you do please let me know 😋

[purple-emily]

I'm currently changing this to be 1:1 functionality with the old, just a simple rewrite.

    MONGO_URI: process.env.MONGODB_URI || 'mongodb://mongo:mongo@localhost:27017/selfhostio?authSource=admin',

becomes:

    MONGODB_HOST: process.env.MONGODB_HOST || 'mongodb',
    MONGODB_PORT: process.env.MONGODB_PORT || '27017',
    MONGODB_DB: process.env.MONGODB_DB || 'selfhostio',
    MONGO_INITDB_ROOT_USERNAME: process.env.MONGO_INITDB_ROOT_USERNAME || 'mongo',
    MONGO_INITDB_ROOT_PASSWORD: process.env.MONGO_INITDB_ROOT_PASSWORD || 'mongo',

I want to remove the defaults, and set a few variables as necessary. Some others can remain optional, and they all need documenting.

Error handling needs adding.

[purple-emily]

Pausing for a few hours. Haven't tested the latest push yet. It should work.

[iPromKnight]

@iPromKnight or @Gabisonfire

Can one of you offer me the way to change:

ScrapeConfiguration__StorageConnectionString=host=postgres;username=postgres;password=postgres;database=selfhostio;

so it will use the

# PostgreSQL

POSTGRES_HOST=postgres

POSTGRES_PORT=5432

POSTGRES_USER=postgres

POSTGRES_PASSWORD=postgres

POSTGRES_DB=selfhostio

variables instead. Ideally I want to strip out all double coded values.

I can see in this file:

https://github.com/Gabisonfire/torrentio-scraper-sh/blob/57f4757541eceda82c553e2a7ca7e7063ffb7ee8/src/producer/Extensions/ServiceCollectionExtensions.cs

and then your comment @iPromKnight:

For the producer, anything in the json files in the Configuration folders can be an env var.

Morning Emily
Sorry out this am for a few hours.

Yeah I will sort that out we'd have to introduce new properties for them in the Models/ScraperConfig file and turn the connection string into a computed property. I'll do later 😃

[iPromKnight]

I'm going to add comments into the code, if we can make a habit of this going forward I'd be a lot happier.

I can see a few undocumented environment variables, like:

const NO_CACHE = process.env.NO_CACHE || false;

I'm curious what the consequences would be if we disabled the cache, is this a functionality we wish to keep or am I safe in assuming this could be removed.

Can I confirm @iPromKnight that you have no plans to drop a 32k commit completely rewriting the addon or consumer 😂😂😂😂😂😂

If you do please let me know 😋

Haha no nothing like that. I'm going to try to see how they run under bun as opposed to nodejs when I get home as it's really gaining traction now

[iPromKnight]

@iPromKnight yeah, I am not well versed in JS so it’s a nightmare. I’m almost learning as I’m going lol. Feel free to pull the branch from my fork and push some changes and PR it 😂

I’m done for today. It’s sit on the sofa and watch crappy TV time

Me neither really. Not used it in years but it's all been coming back this week lol. I much prefer strongly typed typescript.

I've been a .net dev for 23 years though lol

[purple-emily]

@iPromKnight or @Gabisonfire
Can one of you offer me the way to change:

ScrapeConfiguration__StorageConnectionString=host=postgres;username=postgres;password=postgres;database=selfhostio;

so it will use the

# PostgreSQL

POSTGRES_HOST=postgres

POSTGRES_PORT=5432

POSTGRES_USER=postgres

POSTGRES_PASSWORD=postgres

POSTGRES_DB=selfhostio

variables instead. Ideally I want to strip out all double coded values.
I can see in this file:
https://github.com/Gabisonfire/torrentio-scraper-sh/blob/57f4757541eceda82c553e2a7ca7e7063ffb7ee8/src/producer/Extensions/ServiceCollectionExtensions.cs
and then your comment @iPromKnight:
For the producer, anything in the json files in the Configuration folders can be an env var.

Morning Emily Sorry out this am for a few hours.

Yeah I will sort that out we'd have to introduce new properties for them in the Models/ScraperConfig file and turn the connection string into a computed property. I'll do later 😃

Great! You think it’s worth doing the same with RabbitMQ as well? The service you proposed in #45 will need access to Rabbit as well right? Tidy the Rabbit vars up so it’s easier to switch to an unattached service if the user already has one.

I think Mongo and Postgres are done. Just needs testing to make sure nothing is broken.

I am trying to back edit some of the changes you made into the consumer into the add on.

How much of the code from addon/consumer is reused? Could these two be merged into one folder with a different entrypoint.sh/command.

From my quick perusal a lot of the code is duplicated anyway so all we are doing is doubling the maintenance cost.

[purple-emily]

I'm going to add comments into the code, if we can make a habit of this going forward I'd be a lot happier.
I can see a few undocumented environment variables, like:

const NO_CACHE = process.env.NO_CACHE || false;

I'm curious what the consequences would be if we disabled the cache, is this a functionality we wish to keep or am I safe in assuming this could be removed.
Can I confirm @iPromKnight that you have no plans to drop a 32k commit completely rewriting the addon or consumer 😂😂😂😂😂😂
If you do please let me know 😋

Haha no nothing like that. I'm going to try to see how they run under bun as opposed to nodejs when I get home as it's really gaining traction now

I’ll nod along and trust your judgement. The only bun I’m thinking of is 🧁

[purple-emily]

@iPromKnight

Is this a bug? Addon cache is hard coded as: selfhostio_addon_collection

Consumer cache is using cacheConfig.COLLECTION_NAME which is selfhostio_consumer_collection

Should these be the same value?

[iPromKnight]

I'm going to add comments into the code, if we can make a habit of this going forward I'd be a lot happier.
I can see a few undocumented environment variables, like:

const NO_CACHE = process.env.NO_CACHE || false;

I'm curious what the consequences would be if we disabled the cache, is this a functionality we wish to keep or am I safe in assuming this could be removed.
Can I confirm @iPromKnight that you have no plans to drop a 32k commit completely rewriting the addon or consumer 😂😂😂😂😂😂
If you do please let me know 😋

Haha no nothing like that. I'm going to try to see how they run under bun as opposed to nodejs when I get home as it's really gaining traction now

I’ll nod along and trust your judgement. The only bun I’m thinking of is 🧁

Haha 👅

Unfortunately Bun is a no go..
For now at least
3 Hours of fixes getting the database to connect only for it to fail on ingestion as bun doesn't currently implement dgram, so the torrent info extracted doesn't work (no UDP support)
Hey-ho - one day ^^. They have it on the backlog

@iPromKnight

Is this a bug? Addon cache is hard coded as: selfhostio_addon_collection

Consumer cache is using cacheConfig.COLLECTION_NAME which is selfhostio_consumer_collection

Should these be the same value?

No no - the consumer one is just for imdbIds - once its found one for a series etc, it keeps it cached for 7 days so that if it finds another episode or another file in the torrent it needs to look up, it can get the id without having to go off and use name-to-imdb, which is rate limited, or the fall-back google search

Should probably be an env var on the addon though - and not hard coded 😄
I'm actually about to push a PR that updates cache-manager to the latest version, and I definitely agree with the reuse issue

I think for lib - it kinda makes sense to create a shared project, and bring it into both the consumer and addon, get rid of all this duplication.

[purple-emily]

Agree with the shared library idea.

[purple-emily]

Just the producer left to tidy up @iPromKnight to get everything on the same level.

The library idea can be done in another request.

Can merge this as is and create a new pr for producer parity, or do it in this PR

[iPromKnight]

I'll pull down in a few and sort it out cheers Emily

[iPromKnight]

LGTM!