Update MLflow integration #40

AleksanderWWW · 2023-05-02T20:41:42Z

No description provided.

normandy7

Might just be me, but I think "neptune mlflow exporter" sounds like exporting from Neptune to MLflow. Maybe it doesn't matter too much if this string of words is just internal, though.

pyproject.toml

src/neptune_mlflow_exporter/__init__.py

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

CHANGELOG.md

Raalsky · 2023-06-12T11:31:54Z

pyproject.toml

+    "License :: OSI Approved :: Apache Software License",
+    "Natural Language :: English",
+    "Operating System :: MacOS",
+    "Operating System :: Microsoft :: Windows",


Are you sure? We're not testing it on Windows currently 😉

Now we are 😉

Raalsky · 2023-06-12T11:55:20Z

setup.py

-            packages=find_packages(where="src"),
-            platforms="any",
-            install_requires=requirements,
-            entry_points={"neptune.plugins": "mlflow = neptune_mlflow_plugin:sync"},


Do we still support it? I saw a cmdline tool but this syntax looks like a MLFlow plugins triggers or something like that.

Raalsky · 2023-06-12T13:14:36Z

src/neptune_mlflow_exporter/__init__.py

@@ -0,0 +1,64 @@
+#
+# Copyright (c) 2019, Neptune Labs Sp. z o.o.


⌛ 2️⃣ 0️⃣ 2️⃣ 3️⃣ 😜

Raalsky · 2023-06-12T13:19:12Z

src/neptune_mlflow_exporter/impl/sync.py

+import os
+from typing import Optional
+
+import neptune


neptune is optional, don't we wanna return something in case of a lack of package?

checkout neptune_mlflow_exporter/impl/version.py file

Raalsky · 2023-06-12T13:20:48Z

src/neptune_mlflow_exporter/impl/sync.py

+    project_name: Optional[str] = None,
+    api_token: Optional[str] = None,
+    mlflow_tracking_uri: Optional[str] = None,
+    exclude_artifacts: bool = True,


The default in click logic is False: https://github.com/neptune-ai/neptune-mlflow/pull/40/files#diff-ad53078777fa4f7014626a7cad1bee9090623bab90d6888d16d9e0e49ea37906R28

Raalsky · 2023-06-12T15:46:49Z

src/neptune_mlflow_exporter/impl/export.py

+                neptune_run[f"run_data/metrics/{key}"].append(val)
+
+        del data_dict["metrics"]
+        neptune_run["run_data"] = data_dict


Let's log it before the metrics. This line looks really weird if someone doesn't know how the logging works (looks like deleting the run_data/metrics right after we logged the values 😜 ).

src/neptune_mlflow_exporter/impl/utils.py

Raalsky · 2023-06-12T16:36:46Z

src/neptune_mlflow_exporter/impl/utils.py

+    from neptune.new.metadata_containers import Run
+
+
+class ArtifactUploadStrategy(ABC):


This could be simplified by a lot. The logic is much more common for both strategies. In general:

Ensure the size is ok - No need to differentiate as it could be generalized

Download artifact (could be downloaded to some "temp" directory to simplify) - No need to differentiate

Upload file/files (this is the only part that I think needs to depend on artifact type)

Clean the "temp" directory with shutil.rmtree - No need to differentiate

src/neptune_mlflow_exporter/__init__.py

src/neptune_mlflow_exporter/impl/sync.py

src/neptune_mlflow_exporter/impl/neptune_exporter/neptune_exporter.py

Raalsky · 2023-06-21T06:42:20Z

src/neptune_mlflow_exporter/impl/components/fetcher.py

+                max_results=page_limit, page_token=page_token, view_type=ViewType.ACTIVE_ONLY
+            )
+
+            all_experiments.extend(experiments)


How about a Generator here? I mean using yield experiments instead

src/neptune_mlflow_exporter/impl/components/exporter.py

src/neptune_mlflow_exporter/impl/strategies/artifact_strategy.py

clean up repo structure

f04af15

SiddhantSadangi requested a review from AurimasGr May 2, 2023 20:47

AleksanderWWW added 7 commits May 5, 2023 11:57

prototype new exporter structure

75b93b5

export run info; delete unused code

fb2fbc4

start neptune run as context manager

e88e712

use env variables

3a30b63

neptune run ids as set

d329442

check value of max artifact size

dd2cc99

pass tracking uri to mlflow client

7602db0

AleksanderWWW marked this pull request as draft May 5, 2023 17:51

AleksanderWWW added 20 commits May 9, 2023 18:58

artifact listing and iteration

6a1f4f8

convert MB to bytes

b28a245

update gitignore

c350b4d

add tests for sync

1689f26

add tests for export + refactor

8e661f0

refactor test

b127e5a

handle time series metrics

00486ea

implement integration as a class

d65c5c7

var name refactor

feaab96

add tests for NeptuneExporter

5be8fe1

create entry point for neptune-mlflow

7809627

fix entrypoint

b63aab6

update plugin tests

c3c8fd5

split run exporting to two methods

aab424d

refactor _export_run_data

07518ea

change tests dir structure

c8d6a47

add .neptune to gitignore

7bda1c6

update gitignore

4ab775b

add GH actions

bbf5bd9

add project name to constructor

4c55e29

normandy7 requested changes May 25, 2023

View reviewed changes

AleksanderWWW and others added 3 commits May 27, 2023 15:49

Apply suggestions from docs review

bb01655

Co-authored-by: Sabine <sabine.nyholm@neptune.ai>

update doc strings

9e182f0

fix unit test - mlflow client was not patched

5a74d9e

AleksanderWWW requested a review from normandy7 May 31, 2023 18:39

normandy7 previously approved these changes May 31, 2023

View reviewed changes

import from neptune in try-except block

7ff7d2d

AleksanderWWW dismissed normandy7’s stale review via 7ff7d2d June 2, 2023 08:46

AleksanderWWW added 4 commits June 2, 2023 10:53

only remove existing directories

571c45f

add unit tests for utils

1528883

add CHANGELOG

7ce6485

correct changelog

8c5a192

normandy7 reviewed Jun 2, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

spelling of frameworks

c78f2e1

Raalsky reviewed Jun 12, 2023

View reviewed changes

AleksanderWWW added 9 commits June 13, 2023 11:53

fix artifact_startegy

69665c8

test on windows as well

b5edcc5

add and update license headers

d745de9

add version file that also checks if neptune installed

21ab709

split exporter to smaller components

2bfac21

delete bad tests

14c01ea

move version higher up in the catalog tree

bef0ac9

apply review + self-review

abfbcd6

add exception handling to the orchestrator

096247c

Raalsky reviewed Jun 21, 2023

View reviewed changes

AleksanderWWW added 2 commits June 21, 2023 16:38

apply code review suggestions

2badac2

fix tests

1d80316

Raalsky approved these changes Jun 21, 2023

View reviewed changes

AleksanderWWW merged commit e700b83 into master Jun 21, 2023
7 checks passed

AleksanderWWW deleted the aw/update-integration branch June 21, 2023 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update MLflow integration #40

Update MLflow integration #40

AleksanderWWW commented May 2, 2023

normandy7 left a comment

Raalsky Jun 12, 2023

AleksanderWWW Jun 16, 2023

Raalsky Jun 12, 2023

Raalsky Jun 12, 2023

Raalsky Jun 12, 2023

AleksanderWWW Jun 16, 2023

Raalsky Jun 12, 2023

Raalsky Jun 12, 2023

Raalsky Jun 12, 2023

Raalsky Jun 21, 2023

		@@ -0,0 +1,64 @@
		#
		# Copyright (c) 2019, Neptune Labs Sp. z o.o.

		from neptune.new.metadata_containers import Run


		class ArtifactUploadStrategy(ABC):

Update MLflow integration #40

Update MLflow integration #40

Conversation

AleksanderWWW commented May 2, 2023

normandy7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment