Allow configuration of API host

[MDrakos]

https://activestatef.atlassian.net/browse/CP-1050

[MDrakos]

I'm not sure what state the integration tests are in but there are a lot of failures that don't seem related to this changeset at all. If you think I need to investigate any of them let me know.

[mitchell-as]

I can confirm they are all known failures and unrelated to this PR.

[Naatan]

Nice work so far!

[Naatan]

We should route in the config instance from main / prime. Since that controls closing and writing of the config file. I think we'll run into conflicts if we have multiple instances.

[MDrakos]

Hmm this one is a bit awkward. This package is so low-level that passing the config in here will lead to A LOT of cascading changes. Many function signatures will need to be updated to pass a config instance or an interface. I've added a proposal for DB protection in this commit.

We could consider something like above with a lockfile, some sort of global registry of config instances (though this doesn't protect us for inter-process access), bite the bullet and do the updates required to pass the config instance around, or some other solution that I'm not thinking of.

[Naatan]

This would close the lock after the connection has been created, which doesn't solve the potential conflict. In order for the lock to be valid it would have to be closed when the config connection is closed, but if that were the case then your use-case would not work either as it would encounter the lock..

I think this might be a scenario where it makes more sense to break our rule of "no globals" given the application of the package (api) is so global, and not contextual. Perhaps in time we can properly solve this once and for all by threading a context everywhere.

So I'd suggest calling something like api.Init(cfg) from main.go. Just make sure that tests are not using this global, because that would break concurrent tests.

[MDrakos]

Maybe I'm misunderstanding how the lockfile library works but I believe it's accessing the lockfile only for database writes and then releasing it. Which should prevent us from running into conflicts between the state tool and the service. I'm realizing now that I'm attempting to solve a problem that already exists so I'll look into the app.Init(cfg) approach.

[Naatan]

@MDrakos oh also, I don't think this is addressing the following AC:

The state-svc also picks up the new config change

I don't particularly care how, that could be restarting the svc I imagine.

[MDrakos]

@MDrakos oh also, I don't think this is addressing the following AC:

The state-svc also picks up the new config change

I don't particularly care how, that could be restarting the svc I imagine.

Yeah, I tried to call this out in one of the integration tests here. The service will pick up the config change after it restarts, or if the config value was set before the service started. The test above my comment specifically sets the config value before the service starts to try to confirm that.

[Naatan]

@MDrakos oh also, I don't think this is addressing the following AC:

The state-svc also picks up the new config change

I don't particularly care how, that could be restarting the svc I imagine.

Yeah, I tried to call this out in one of the integration tests here. The service will pick up the config change after it restarts, or if the config value was set before the service started. The test above my comment specifically sets the config value before the service starts to try to confirm that.

I could have been more explicit in writing that AC, but the intend is that it picks up the change as it's made, without the user manually restarting the state-svc. It should not require the user to manually restart the svc. If this turns out to be too deep a rabbit hole we can probably descope.

[Naatan]

Looking good! Just some churn on the svc restarting stuff -- #3717 (comment)

[MDrakos]

Okay with the latest changes the state-svc should pick up when the api host changes. However, because most of our service clients are constructed with a host the state-svc may not change where its requests are going. This would require a larger refactor of a bunch of services to be sure or some sort of restart mechanism when this config value changes.

You have another story for the installers to support config options. If the state tool is installed with the config options set then this will not be an issue. Otherwise it will require a state-svc restart.

[Naatan]

Looking good, nearly there!

[Naatan]

Nice work!