Merge pull request #37 from pebeto/improving_code_base

Improving code base

.github/codecov.yml

@@ -0,0 +1,2 @@
+ignore:
+- "src/deprecated.jl"

docs/make.jl

@@ -17,7 +17,8 @@ makedocs(;
         "Home" => "index.md",
         "Tutorial" => "tutorial.md",
         "Reference" => "reference.md"
-    ]
+    ],
+    checkdocs=:exports
 )
 
 deploydocs(;

docs/src/reference.md

@@ -14,6 +14,7 @@ MLFlowExperiment
 MLFlowRun
 MLFlowRunInfo
 MLFlowRunData
+MLFlowRunDataParam
 MLFlowRunDataMetric
 MLFlowRunStatus
 MLFlowArtifactFileInfo
@@ -28,7 +29,7 @@ getexperiment
 getorcreateexperiment
 deleteexperiment
 searchexperiments
-listexperiments
+restoreexperiment
 ```
 
 # Runs
@@ -55,5 +56,5 @@ uri
 generatefilterfromentity_type
 generatefilterfromparams
 generatefilterfromattributes
-
+healthcheck
 ```

src/MLFlowClient.jl

@@ -20,12 +20,15 @@ using JSON
 using ShowCases
 using FilePathsBase: AbstractPath
 
-include("types/core.jl")
+include("types/mlflow.jl")
+export
+    MLFlow
+
+include("types/experiment.jl")
 export
-    MLFlow,
     MLFlowExperiment
 
-include("types/runs.jl")
+include("types/run.jl")
 export
     MLFlowRunStatus,
     MLFlowRunInfo,
@@ -38,13 +41,15 @@ export
     get_run_id,
     get_params
 
-include("types/artifacts.jl")
+include("types/artifact.jl")
 export
     MLFlowArtifactFileInfo,
     MLFlowArtifactDirInfo,
     get_path,
     get_size
 
+include("api.jl")
+
 include("utils.jl")
 export
     generatefilterfromparams
@@ -75,4 +80,7 @@ export
     logmetric,
     logartifact,
     listartifacts
+
+include("deprecated.jl")
+
 end

src/api.jl

@@ -0,0 +1,34 @@
+"""
+    mlfget(mlf, endpoint; kwargs...)
+
+Performs a HTTP GET to a specified endpoint. kwargs are turned into GET params.
+"""
+function mlfget(mlf, endpoint; kwargs...)
+    apiuri = uri(mlf, endpoint, kwargs)
+    apiheaders = headers(mlf, Dict("Content-Type" => "application/json"))
+
+    try
+        response = HTTP.get(apiuri, apiheaders)
+        return JSON.parse(String(response.body))
+    catch e
+        throw(e)
+    end
+end
+
+"""
+    mlfpost(mlf, endpoint; kwargs...)
+
+Performs a HTTP POST to the specified endpoint. kwargs are converted to JSON and become the POST body.
+"""
+function mlfpost(mlf, endpoint; kwargs...)
+    apiuri = uri(mlf, endpoint)
+    apiheaders = headers(mlf, Dict("Content-Type" => "application/json"))
+    body = JSON.json(kwargs)
+
+    try
+        response = HTTP.post(apiuri, apiheaders, body)
+        return JSON.parse(String(response.body))
+    catch e
+        throw(e)
+    end
+end

src/deprecated.jl

@@ -7,6 +7,6 @@ Deprecated (last MLFlow version: 1.30.1) in favor of [`searchexperiments`](@ref)
 """
 
 function listexperiments(mlf::MLFlow)
-endpoint = "experiments/list"
+    endpoint = "experiments/list"
     mlfget(mlf, endpoint)
 end

src/experiments.jl

@@ -15,12 +15,24 @@ An object of type [`MLFlowExperiment`](@ref).
 """
 function createexperiment(mlf::MLFlow; name=missing, artifact_location=missing, tags=missing)
     endpoint = "experiments/create"
+
     if ismissing(name)
         name = string(UUIDs.uuid4())
     end
-    result = mlfpost(mlf, endpoint; name=name, artifact_location=artifact_location, tags=tags)
-    experiment_id = parse(Int, result["experiment_id"])
-    getexperiment(mlf, experiment_id)
+
+    try
+        result = mlfpost(mlf, endpoint; name=name, artifact_location=artifact_location, tags=tags)
+        experiment_id = parse(Int, result["experiment_id"])
+        return getexperiment(mlf, experiment_id)
+    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
+        end
+        throw(e)
+    end
 end
 
 """
@@ -38,7 +50,9 @@ An instance of type [`MLFlowExperiment`](@ref)
 """
 function getexperiment(mlf::MLFlow, experiment_id::Integer)
     try
-        result = _getexperimentbyid(mlf, experiment_id)
+        endpoint = "experiments/get"
+        arguments = (:experiment_id => experiment_id,)
+        result = mlfget(mlf, endpoint; arguments...)["experiment"]
         return MLFlowExperiment(result)
     catch e
         if isa(e, HTTP.ExceptionRequest.StatusError) && e.status == 404
@@ -62,7 +76,9 @@ An instance of type [`MLFlowExperiment`](@ref)
 """
 function getexperiment(mlf::MLFlow, experiment_name::String)
     try
-        result = _getexperimentbyname(mlf, experiment_name)
+        endpoint = "experiments/get-by-name"
+        arguments = (:experiment_name => experiment_name,)
+        result = mlfget(mlf, endpoint; arguments...)["experiment"]
         return MLFlowExperiment(result)
     catch e
         if isa(e, HTTP.ExceptionRequest.StatusError) && e.status == 404
@@ -71,16 +87,6 @@ function getexperiment(mlf::MLFlow, experiment_name::String)
         throw(e)
     end
 end
-function _getexperimentbyid(mlf::MLFlow, experiment_id::Integer)
-    endpoint = "experiments/get"
-    arguments = (:experiment_id => experiment_id,)
-    mlfget(mlf, endpoint; arguments...)["experiment"]
-end
-function _getexperimentbyname(mlf::MLFlow, experiment_name::String)
-    endpoint = "experiments/get-by-name"
-    arguments = (:experiment_name => experiment_name,)
-    mlfget(mlf, endpoint; arguments...)["experiment"]
-end
 
 """
     getorcreateexperiment(mlf::MLFlow, experiment_name::String; artifact_location=missing, tags=missing)
@@ -98,11 +104,12 @@ An instance of type [`MLFlowExperiment`](@ref)
 
 """
 function getorcreateexperiment(mlf::MLFlow, experiment_name::String; artifact_location=missing, tags=missing)
-    exp = getexperiment(mlf, experiment_name)
-    if ismissing(exp)
-        exp = createexperiment(mlf, name=experiment_name, artifact_location=artifact_location, tags=tags)
+    experiment = getexperiment(mlf, experiment_name)
+
+    if ismissing(experiment)
+        return createexperiment(mlf, name=experiment_name, artifact_location=artifact_location, tags=tags)
     end
-    exp
+    return experiment
 end
 
 """
@@ -122,14 +129,14 @@ function deleteexperiment(mlf::MLFlow, experiment_id::Integer)
     endpoint = "experiments/delete"
     try
         mlfpost(mlf, endpoint; experiment_id=experiment_id)
+        return true
     catch e
         if isa(e, HTTP.ExceptionRequest.StatusError) && e.status == 404
             # experiment already deleted
             return true
         end
         throw(e)
     end
-    true
 end
 
 """
@@ -164,14 +171,16 @@ function restoreexperiment(mlf::MLFlow, experiment_id::Integer)
     endpoint = "experiments/restore"
     try
         mlfpost(mlf, endpoint; experiment_id=experiment_id)
+        return true
     catch e
         if isa(e, HTTP.ExceptionRequest.StatusError) && e.status == 404
-            # experiment already restored
-            return true
+            error_code = JSON.parse(String(e.response.body))["error_code"]
+            if error_code == MLFLOW_ERROR_CODES.RESOURCE_DOES_NOT_EXIST
+                error("Experiment with id \"$experiment_id\" does not exist")
+            end
         end
         throw(e)
     end
-    true
 end
 
 restoreexperiment(mlf::MLFlow, experiment::MLFlowExperiment) =

src/types/core.jl → src/types/experiment.jl

@@ -1,39 +1,3 @@
-"""
-    MLFlow
-
-Base type which defines location and version for MLFlow API service.
-
-# Fields
-- `baseuri::String`: base MLFlow tracking URI, e.g. `http://localhost:5000`
-- `apiversion`: used API version, e.g. `2.0`
-- `headers`: HTTP headers to be provided with the REST API requests (useful for authetication tokens)
-
-# Constructors
-
-- `MLFlow(baseuri; apiversion=2.0,headers=Dict())`
-- `MLFlow()` - defaults to `MLFlow("http://localhost:5000")`
-
-# Examples
-
-```@example
-mlf = MLFlow()
-```
-
-```@example
-remote_url="https://<your-server>.cloud.databricks.com"; # address of your remote server
-mlf = MLFlow(remote_url, headers=Dict("Authorization" => "Bearer <your-secret-token>"))
-```
-
-"""
-struct MLFlow
-    baseuri::String
-    apiversion
-    headers::Dict
-end
-MLFlow(baseuri; apiversion=2.0,headers=Dict()) = MLFlow(baseuri, apiversion,headers)
-MLFlow() = MLFlow("http://localhost:5000", 2.0, Dict())
-Base.show(io::IO, t::MLFlow) = show(io, ShowCase(t, [:baseuri,:apiversion], new_lines=true))
-
 """
     MLFlowExperiment
 

src/types/mlflow.jl

@@ -0,0 +1,35 @@
+"""
+    MLFlow
+
+Base type which defines location and version for MLFlow API service.
+
+# Fields
+- `baseuri::String`: base MLFlow tracking URI, e.g. `http://localhost:5000`
+- `apiversion`: used API version, e.g. `2.0`
+- `headers`: HTTP headers to be provided with the REST API requests (useful for authetication tokens)
+
+# Constructors
+
+- `MLFlow(baseuri; apiversion=2.0,headers=Dict())`
+- `MLFlow()` - defaults to `MLFlow("http://localhost:5000")`
+
+# Examples
+
+```@example
+mlf = MLFlow()
+```
+
+```@example
+remote_url="https://<your-server>.cloud.databricks.com"; # address of your remote server
+mlf = MLFlow(remote_url, headers=Dict("Authorization" => "Bearer <your-secret-token>"))
+```
+
+"""
+struct MLFlow
+    baseuri::String
+    apiversion::Union{Integer, AbstractFloat}
+    headers::Dict
+end
+MLFlow(baseuri; apiversion=2.0,headers=Dict()) = MLFlow(baseuri, apiversion,headers)
+MLFlow() = MLFlow("http://localhost:5000", 2.0, Dict())
+Base.show(io::IO, t::MLFlow) = show(io, ShowCase(t, [:baseuri,:apiversion], new_lines=true))

src/types/runs.jl → src/types/run.jl

@@ -127,7 +127,6 @@ Represents a parameter.
 - `MLFlowRunDataParam(d::Dict{String,String})`
 
 """
-
 struct MLFlowRunDataParam
     key::String
     value::String

src/utils.jl

@@ -42,39 +42,6 @@ headers(mlf,Dict("Content-Type"=>"application/json"))
 """
 headers(mlf::MLFlow, custom_headers::AbstractDict) = merge(mlf.headers, custom_headers)
 
-"""
-    mlfget(mlf, endpoint; kwargs...)
-
-Performs a HTTP GET to a specifid endpoint. kwargs are turned into GET params.
-"""
-function mlfget(mlf, endpoint; kwargs...)
-    apiuri = uri(mlf, endpoint, kwargs)
-    apiheaders = headers(mlf, Dict("Content-Type" => "application/json"))
-    try
-        response = HTTP.get(apiuri, apiheaders)
-        return JSON.parse(String(response.body))
-    catch e
-        throw(e)
-    end
-end
-
-"""
-    mlfpost(mlf, endpoint; kwargs...)
-
-Performs a HTTP POST to the specified endpoint. kwargs are converted to JSON and become the POST body.
-"""
-function mlfpost(mlf, endpoint; kwargs...)
-    apiuri = uri(mlf, endpoint)
-    apiheaders = headers(mlf, Dict("Content-Type" => "application/json"))
-    body = JSON.json(kwargs)
-    try
-        response = HTTP.post(apiuri, apiheaders, body)
-        return JSON.parse(String(response.body))
-    catch e
-        throw(e)
-    end
-end
-
 """
     generatefilterfromentity_type(filter_params::AbstractDict{K,V}, entity_type::String) where {K,V}
 
@@ -99,5 +66,21 @@ function generatefilterfromentity_type(filter_params::AbstractDict{K,V}, entity_
     filters = ["$(entity_type).\"$(k)\" = \"$(v)\"" for (k, v) ∈ filter_params]
     join(filters, " and ")
 end
+
+"""
+    generatefilterfromparams(filter_params::AbstractDict{K,V}) where {K,V}
+
+Generates a `filter` string from `filter_params` dictionary and `param` entity type.
+"""
 generatefilterfromparams(filter_params::AbstractDict{K,V}) where {K,V} = generatefilterfromentity_type(filter_params, "param")
+"""
+    generatefilterfrommattributes(filter_attributes::AbstractDict{K,V}) where {K,V}
+
+Generates a `filter` string from `filter_attributes` dictionary and `attribute` entity type.
+"""
 generatefilterfromattributes(filter_attributes::AbstractDict{K,V}) where {K,V} = generatefilterfromentity_type(filter_attributes, "attribute")
+
+const MLFLOW_ERROR_CODES = (;
+    RESOURCE_ALREADY_EXISTS = "RESOURCE_ALREADY_EXISTS",
+    RESOURCE_DOES_NOT_EXIST = "RESOURCE_DOES_NOT_EXIST",
+)

test/test_experiments.jl

@@ -3,6 +3,8 @@
     exp = createexperiment(mlf)
 
     @test isa(exp, MLFlowExperiment)
+    @test_throws ErrorException createexperiment(mlf; name=exp.name)
+
     deleteexperiment(mlf, exp)
 end