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

Integrate with DagsHub's MLflow API #35

Open
math4mad opened this issue Jan 7, 2024 · 6 comments
Open

Integrate with DagsHub's MLflow API #35

math4mad opened this issue Jan 7, 2024 · 6 comments

Comments

@math4mad
Copy link

math4mad commented Jan 7, 2024

According with this part
dagshub mlflow integrations

how to fill params of headers

mlf = MLFlow("https://dagshub.com/math4mad/mlj-test.mlflow";headers=Dict("MLFLOW_TRACKING_USERNAME"=>"******","MLFLOW_TRACKING_PASSWORD"=>"********" ))

# Initiate new experiment
experiment_id = createexperiment(mlf; name="price-paths")

# Create a run in the new experiment
exprun = createrun(mlf, experiment_id)

follow Set-up your credentials setting name and token , now it not working

@pebeto
Copy link
Member

pebeto commented Jan 7, 2024

Hello @math4mad.

That's not the way to authenticate into an mlflow instance. To achieve this, we need to have our base64-encoded credentials and pass it through the authorization header.

# "credentials" is a base64-encoded string
credentials = "<your_username>:<your_password>"
MLFlow(<your_base_uri>; headers=Dict("Authorization" => "Basic $credentials"))

This is an ugly way to send authorizations. I'm going to make it pretty.


However, you can't use this package because it is supporting the modern /api endpoint instead of /ajax-api (the one Dagshub is using). I just reviewed that the last mentioned is used in almost every scenario, so I will adapt that to ensure the compatibility.

@pebeto
Copy link
Member

pebeto commented Jan 7, 2024

This last change is using the generic /ajax-api prefix on each endpoint to allow compatibility with different platforms.

Notes:

  • Original MLFlow instances use base64 to encode authorization headers (based on the Basic Authorization defined here)
  • During my testing, Dagshub is allowing both raw and base64-encoded username and password.

I'm thinking about improving the way we are defining credentials:

  • username and password can be passed in a NamedTuple directly by an auth parameter in MLFlow type constructor. The same case if we are using a token (it becomes a Bearer Auth). In the headers function, we are going to do checks and logic to add that info to the request.
## Old way
using Base64
using MLFlowClient

credentials = base64encode("missy:gala")
MLFlow("http://localhost:5000"; headers=Dict("Authorization"=>"Basic $credentials"))

## New way
using MLFlowClient

MLFlow("http://localhost:5000"; auth=("username"=>"missy", "password"=>"gala"))

@ablaom @deyandyankov

@ablaom
Copy link
Member

ablaom commented Jan 9, 2024

Thanks @pebeto for taking the lead here.

Your suggestion sounds reasonable to me, but would be good to get @deyandyankov's view. Are you adding auth as a new keyword but keeping headers for passing other stuff? Or removing headers altogether?

Instead of auth, I suggest authorisation - abbreviations are hindrance to non-English speakers and sometimes difficult to remember

@ablaom
Copy link
Member

ablaom commented Jan 16, 2024

@deyandyankov Can you please comment on @pebeto's proposal?

@deyandyankov
Copy link
Collaborator

I think having both authorisation and headers makes sense. Probably need to consider the case when both authorisation and headers=Dict("Authorization"=>"Basic $credentials")) have been supplied, but that would be a user error anyway.

@math4mad
Copy link
Author

@pebeto

at now still can't work with remote url of dagshub. could give me a hint?
using python api it is ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants