Add AddQueue to the Client

[PumpkinSeed]

Closes #396

Based on the proposal I added AddQueue/RemoveQueue. This won't do anything just add the queues, so we need to "Restart" the client after that.

What do you think about to add StartWorkContext in the AddQueue section

if err := producer.StartWorkContext(fetchCtx, workCtx); err != nil {
    stopProducers()
    stopServicesOnError()
    return err
}

[PumpkinSeed]

I agree with the reduction of the scope of the PR. I will focus on AddQueue and remove the RemoveQueue, I will address it in a next PR.

[PumpkinSeed]

Is there any documentation about setting up development environment? I'm using Arch Linux and it would be nice to use postgres image/container for testing. I found docs/development.md but most of these using the raw postgres client, however it would be nice to set an env var or something like that so we can customize the postgres connection.

[PumpkinSeed]

Testing notes

I checked the CI and I tried to replicate it, based on this I have a few notes.

I created a postgres docker container:

docker run --name river-postgres -e POSTGRES_PASSWORD=postgres -p 5432:5432 -d postgres

I added the following environment variables:

export PGHOST: 127.0.0.1
export PGPORT: 5432
export PGUSER: postgres
export PGPASSWORD: postgres
export PGSSLMODE: disable
export ADMIN_DATABASE_URL: postgres://postgres:postgres@localhost:5432?sslmode=disable
export DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/river_dev?sslmode=disable
export TEST_DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/river_testdb?sslmode=disable

I use Goland so there is a one-liner:

PGHOST=127.0.0.1;PGPORT=5432;PGUSER=postgres;PGPASSWORD=postgres;PGSSLMODE=disable;ADMIN_DATABASE_URL=postgres://postgres:postgres@localhost:5432?sslmode=disable;DATABASE_URL=postgres://postgres:postgres@127.0.0.1:5432/river_dev?sslmode=disable;TEST_DATABASE_URL=postgres://postgres:postgres@127.0.0.1:5432/river_testdb?sslmode=disable

I changed the managementDatabaseURL in the ./internal/cmd/testdbman/main.go:

const managementDatabaseURL = "postgresql://postgres:postgres@localhost:5432/postgres"

After this I run the database creation/migration detailed in the docs/development.md

go run ./internal/cmd/testdbman create

[PumpkinSeed]

I added the requested changes.

I still have a question whether I need to return s.context in the StartInit at the following section or not:

if s.started {
    return ctx, false, nil
}

[PumpkinSeed]

@brandur @bgentry Do you have any update on this?

[brandur]

Hey @PumpkinSeed, thanks for the updates here.

This is a bit of a tricky change, and there's quite a bit of nuance that's quite subtle — e.g. believe it or not, the way you're checking for IsStarted here is racy because a shutdown could have been finished between the check on IsStarted and the call to StartWorkContext. There's also a couple of other changes need like the addition of a Queues() bundle like I mentioned above.

Do you mind if we take this one over to get it over the finish line?

[PumpkinSeed]

Do you mind if we take this one over to get it over the finish line?

I'm totally fine with it. Thanks for that.

[brandur]

Opened #410 as an alternative.

[brandur]

Closing in favor of #410.