Director readiness probe isn't great about charts HOT 10 CLOSED

tl;dr you can't have a http readiness probe checking for a redirect, it must get a 200 response.
So we should check for something else or not have one.

from charts.

I have removed the probe for now to stop the flapping, but I'll leave this open for now... maybe there's a sensible exec command to use instead.

from charts.

Hello ! I think we could use the mongo db health check I implemented : sorry-cypress/sorry-cypress#589

This way the director will be considered ready when the express server is listening and the connexion with MongoDB is working.

from charts.

This would (obviously) only be a solution if the user has chosen to use a mongodb though.

from charts.

Then I think we should just implement an API health check that respond a 200 if the API is running

from charts.

Is there really an option to not use a MongoDB though ? 🤔 Of course you can fork the project and implement a new ExecutionDriver but it means you rewrite almost half the codebase

from charts.

Ah yeh. It's still technically possible.. you just don't get an API.

And executionDriver: "../execution/in-memory" is valid.

from charts.

The health check is not specific to the mongodb. Your execution driver must implement a isDbHealthy method (which returns true immediately when you use the in-memory driver)

app.get('/health-check-db', catchRequestHandlerErrors(async (_, res) => { const executionDriver = await getExecutionDriver(); (await executionDriver.isDBHealthy()) ? res.sendStatus(200) : res.sendStatus(503); }) );

So I guess using this as a readyness probe can work. Or else we just write something like this :

app.get( '/health-check', (req, res) => { res.sendStatus(200); } );

from charts.

Sounds possible then. I probably can't get to this this week, will happily accept PRs :)

from charts.

Resoved by PR #134

from charts.