Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mlflow health endpoint is doutbful #34

Closed
pebeto opened this issue Jan 21, 2024 · 3 comments · Fixed by #35
Closed

mlflow health endpoint is doutbful #34

pebeto opened this issue Jan 21, 2024 · 3 comments · Fixed by #35
Assignees
Labels
breaking bug Something isn't working

Comments

@pebeto
Copy link
Member

pebeto commented Jan 21, 2024

mlflow local instances provide /health and /ping endpoints to check if it is running. Platforms like DagsHub don't have it (well, as far as I have been able to test), making users being unable to use this package. Should we remove that check?

@pebeto pebeto added bug Something isn't working breaking labels Jan 21, 2024
@pebeto
Copy link
Member Author

pebeto commented Jan 22, 2024

If we are not going to remove this check, we must search a way to detect if the provided URI is a valid mlflow instance.

@ablaom
Copy link
Member

ablaom commented Jan 22, 2024

Just to be clear, are you saying the current health checks don't use public API?

And this has gotten us into trouble, because our checks are crashing sessions via DagsHub?

If the answer to both questions is "yes" then I agree we need to remove the check. In any case, this ought to be something we implement at MLFlowClient.jl, yes? I don't how to officially check instance health, but we could post a question at mljflow.

@pebeto
Copy link
Member Author

pebeto commented Jan 23, 2024

The current healthcheck is using a non REST endpoint to show mlflow instance status. This endpoint is enabled by default with the common distribution. However, platforms like DagsHub removed it by unknown reasons. So, yes to both questions.

We need to request a healthcheck inside the REST API to have an official way to do this. If that's not the case, we can play with the webpage to extract certain parts to ensure an instance (but that's awful).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants