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

Setting ./mlruns as the default artifact_location #49

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ concurrency:
cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }}
jobs:
test:

services:
mlflow:
image: adacotechjp/mlflow:2.3.1
ports:
- 5000:5000

name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }}
runs-on: ${{ matrix.os }}
strategy:
Expand Down Expand Up @@ -48,6 +46,10 @@ jobs:
${{ runner.os }}-test-
${{ runner.os }}-
- uses: julia-actions/julia-buildpkg@v1
- name: Creating a dummy file to test logartifacts
run: |
touch ./test/mlflowclient-tempfile.txt
realpath ./test/mlflowclient-tempfile.txt
- uses: julia-actions/julia-runtest@v1
env:
MLFLOW_TRACKING_URI: "http://localhost:5000/api"
Expand Down
8 changes: 5 additions & 3 deletions src/experiments.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ Creates an MLFlow experiment.
# Arguments
- `mlf`: [`MLFlow`](@ref) configuration.
- `name`: experiment name. If not specified, MLFlow sets it.
- `artifact_location`: directory where artifacts of this experiment will be stored. If not specified, MLFlow uses its default configuration.
- `artifact_location`: directory where artifacts of this experiment will be
stored. If not specified, `./mlruns` will be used (it will take the directory
where you are running `mlflow` as the root one).
- `tags`: a Dictionary of key-values which tag the experiment.

# Returns
An object of type [`MLFlowExperiment`](@ref).

"""
function createexperiment(mlf::MLFlow; name=missing, artifact_location=missing, tags=missing)
function createexperiment(mlf::MLFlow; name=missing, artifact_location="./mlruns", tags=missing)
endpoint = "experiments/create"

if ismissing(name)
Expand Down Expand Up @@ -103,7 +105,7 @@ Gets an experiment if one alrady exists, or creates a new one.
An instance of type [`MLFlowExperiment`](@ref)

"""
function getorcreateexperiment(mlf::MLFlow, experiment_name::String; artifact_location=missing, tags=missing)
function getorcreateexperiment(mlf::MLFlow, experiment_name::String; artifact_location="./mlruns", tags=missing)
experiment = getexperiment(mlf, experiment_name)

if ismissing(experiment)
Expand Down
17 changes: 13 additions & 4 deletions test/test_loggers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ end
end

@testset "logartifact_error" begin
@test_broken logartifact(mlf, r, "/etc/shadow")
@test_broken logartifact(mlf, r, "/etc/misina")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a @test_throws ? Usually @test_broken is for temporarilydisabling tests that are failing but which we hope to rectify in the near future.

end

deleteexperiment(mlf, e)
Expand Down Expand Up @@ -167,20 +167,29 @@ end
runname = "run-$(UUIDs.uuid4())"
r = createrun(mlf, e.experiment_id)

tempfilename = "./mlflowclient-tempfile.txt"

open(tempfilename, "w") do file
write(file, "Hello, world!\n")
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is more usual to use the functions tempname or tempdir to create temporary files during CI, although I think this still works. But perhaps you have reasons for doing it this way?

logartifact(mlf, r, tempfilename)

@testset "listartifacts_by_run_id" begin
artifacts = listartifacts(mlf, r.info.run_id)
@test length(artifacts) == 0
@test length(artifacts) == 1
end

@testset "listartifacts_by_run" begin
artifacts = listartifacts(mlf, r)
@test length(artifacts) == 0
@test length(artifacts) == 1
end

@testset "listartifacts_by_run_info" begin
artifacts = listartifacts(mlf, r.info)
@test length(artifacts) == 0
@test length(artifacts) == 1
end

rm(tempfilename)
deleteexperiment(mlf, e)
end
Loading