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

Throw more readable HTTP error. #58

Closed
NeroBlackstone opened this issue Oct 28, 2024 · 2 comments
Closed

Throw more readable HTTP error. #58

NeroBlackstone opened this issue Oct 28, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@NeroBlackstone
Copy link

NeroBlackstone commented Oct 28, 2024

For now, the MLFlowClient.jl simply throw out HTTP.Exceptions.StatusError or use hard code error message, and this is hard to figuar out what is wrong.

I found that for any 4xx and 5xx JSON response, it will give a error message:

{
    "error_code": "RESOURCE_DOES_NOT_EXIST",
    "message": "Could not find experiment with ID 0004"
}

So, why not parse and throw this error message , rather than manually hard code error message like:

 catch e
        if isa(e, HTTP.ExceptionRequest.StatusError) && e.status == 400
            error_code = JSON.parse(String(e.response.body))["error_code"]
           # if error_code == MLFLOW_ERROR_CODES.RESOURCE_ALREADY_EXISTS
           #     error("Experiment with name \"$name\" already exists")
           #end
           error_message = JSON.parse(String(e.response.body))["error_message"]
           error(error_message)
        end
        throw(e)
    end

We can add a unified error handler, if you think this is useful I can implement it and open a pull request!

Thanks!

@pebeto pebeto added enhancement New feature or request good first issue Good for newcomers labels Oct 29, 2024
@pebeto
Copy link
Member

pebeto commented Oct 29, 2024

Hi @NeroBlackstone,

Thanks for your contribution. Sure, it is something to take in count. I'm going to put it on my queue (but it's open to anyone).

@pebeto
Copy link
Member

pebeto commented Nov 3, 2024

A solution was implemented in #59.

@pebeto pebeto closed this as completed Nov 3, 2024
@pebeto pebeto self-assigned this Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants