JuliaAI
Adding `use_ajax` flag to `MLFlow` instances
Are there any other MLFlow implementations/deployments using /ajax-api other than DagsHub? (I have only seen mention of /ajax-api in https://github.com/JuliaAI/MLFlowClient.jl/issues/35#issuecomment-1880158336)
Wouldn't a more natural API be to provide the API base url, e.g. http://localhost:5000/api or https://dagshub.com/nirbarazida/CheXNet.mlflow/ajax-api (?), instead of providing the MLFlow UI base url (http://localhost:5000) and having a kwarg with a not-very-descriptive name (use_ajax) ?
I don't know if there's other than DagsHub.
I consider the API base URL approach as a good one. With that, we don't need to be aware of if it's an ajax-api or api.
Need opinions about this decision. @ablaom @deyandyankov
I agree with the base URL approach, but it will break compatibility for existing users. I.e. they will need to update their URLs if we make that change and they upgrade to a newer release of the package.
No objections but this definitely needs to be tagged breaking. So if we have any other breaking changes in mind...
I'm very excited about this PR! I was about to open a new PR myself, as the latest version is broken for us (past ones work).
The following commit broke the ability to work with Databricks: https://github.com/JuliaAI/MLFlowClient.jl/commit/e27ac1026627327d58fd4b63aeeef62c738b0977
We use Databricks-managed MLFlow and we need the "HOST/api/2.0" format rather than the current "/ajax-api" one.
The proposed approach to provide the base URL including the relevant API path would work great for us!
No objections but this definitely needs to be tagged breaking. So if we have any other breaking changes in mind...
A very much related breaking change would be to change the type of apiversion::Union{Integer, AbstractFloat} to, e.g. api_version::VersionNumber.
@pebeto Would it be, please, possible to merge this PR and tag a new version?
Given that the commit linked above was breaking (anything above v0.4.4), anyone who updates (even if having compat to "0.4") might get the same error as we did.
It would be good to have a clean new tag.
@pebeto Would you please consider merging the PR to have a working minor version release?
The bug introduced in 0.4.5 (the ajax in path) still persists.