SolarEdgeTech/pyctuator

health-Endpoint: Does not return 503 when Status is DOWN

thurse93 opened this issue · 5 comments

I have the following minimal example:

from dataclasses import dataclass

from fastapi import FastAPI
from pyctuator.health.health_provider import HealthDetails, HealthProvider, HealthStatus, Status
from pyctuator.pyctuator import Pyctuator

@dataclass
class AppSpecificHealthDetails(HealthDetails):
    my_dependency_connectivity: str


class MyHealthProvider(HealthProvider):
    def is_supported(self) -> bool:
        return True

    def get_name(self) -> str:
        return "my-dependency-health"

    def get_health(self) -> HealthStatus:
        return HealthStatus(
            status=Status.DOWN,
            details=AppSpecificHealthDetails(my_dependency_connectivity="is down")
        )


app = FastAPI()


@app.get("/")
async def hello_world():
    return {"message": "hello world"}

pyctuator = Pyctuator(
    app,
    app_name="my_app",
    app_url="http://127.0.0.1:8000/",
    pyctuator_endpoint_url="http://127.0.0.1:8000/pyctuator",
    registration_url=None
)

pyctuator.register_health_provider(MyHealthProvider())

As expected the Status of the Subcomponent is DOWN, and subsequently the status of the whole application is also DOWN. However the status_code is still 200. I would expect to receive 503 for Status codes DOWN, just like in a Spring Boot Application.

$ curl -vs http://localhost:8000/pyctuator/health
{"status":"DOWN","details":{"my-dependency-health":{"status":"DOWN","details":{"my_dependency_connectivity":"is down"}}}}*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /pyctuator/health HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.81.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< date: Fri, 02 Sep 2022 08:51:27 GMT
< server: uvicorn
< content-length: 121
< content-type: application/vnd.spring-boot.actuator.v2+json;charset=UTF-8
<
{ [121 bytes data]
* Connection #0 to host localhost left intact

Will look into this and will also upgrade the dependencies to latest.

Hi @thurse93, the fix is small - see the changes in #83

However, note that this PR is based on a complete dependencies refresh (I had hard times going back to the old python even when using pyenv).

Hey @michaelyaakoby looks good to me, just made a small code comment.

Hi @thurse93 , can you point to your code comment? I can't find it...
Thanks

@michaelyaakoby my bad, I didn't commit it: #83 (comment)