From 3c396b1f7ea22b2f77c680549b5d9df2f0e12ebd Mon Sep 17 00:00:00 2001 From: allegroai <> Date: Thu, 12 May 2022 23:46:32 +0300 Subject: [PATCH] Fix pipelines can't handle `None` return value --- clearml/automation/controller.py | 3 +++ clearml/binding/artifacts.py | 42 ++++++++++++++++---------------- clearml/storage/helper.py | 7 ------ 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/clearml/automation/controller.py b/clearml/automation/controller.py index 74661ca2..18532365 100644 --- a/clearml/automation/controller.py +++ b/clearml/automation/controller.py @@ -4,6 +4,7 @@ import inspect import json import os import re +import six from copy import copy, deepcopy from datetime import datetime from logging import getLogger @@ -2032,6 +2033,8 @@ class PipelineController(object): for g in pattern.findall(value): # update with actual value new_val = self.__parse_step_reference(g) + if not isinstance(new_val, six.string_types): + return new_val updated_value = updated_value.replace(g, new_val, 1) return updated_value diff --git a/clearml/binding/artifacts.py b/clearml/binding/artifacts.py index 6c22d352..bbab5533 100644 --- a/clearml/binding/artifacts.py +++ b/clearml/binding/artifacts.py @@ -48,6 +48,8 @@ class Artifact(object): Read-Only Artifact object """ + _not_set = object() + @property def url(self): # type: () -> str @@ -136,7 +138,7 @@ class Artifact(object): self._metadata = dict(artifact_api_object.display_data) if artifact_api_object.display_data else {} self._preview = artifact_api_object.type_data.preview if artifact_api_object.type_data else None self._content_type = artifact_api_object.type_data.content_type if artifact_api_object.type_data else None - self._object = None + self._object = self._not_set def get(self, force_download=False): # type: (bool) -> Any @@ -156,7 +158,7 @@ class Artifact(object): :param bool force_download: download file from remote even if exists in local cache :return: One of the following objects Numpy.array, pandas.DataFrame, PIL.Image, dict (json), or pathlib2.Path. """ - if self._object: + if self._object is not self._not_set: return self._object local_file = self.get_local_copy(raise_on_error=True, force_download=force_download) @@ -200,9 +202,8 @@ class Artifact(object): ) ) - local_file = Path(local_file) - - if self._object is None: + if self._object is self._not_set: + local_file = Path(local_file) self._object = local_file return self._object @@ -640,7 +641,7 @@ class Artifacts(object): artifact_type_data.content_type = mimetypes.guess_type(artifact_object)[0] if preview: artifact_type_data.preview = preview - elif isinstance(artifact_object, six.string_types): + elif isinstance(artifact_object, six.string_types) and artifact_object: # if we got here, we should store it as text file. artifact_type = 'string' artifact_type_data.content_type = 'text/plain' @@ -652,23 +653,22 @@ class Artifacts(object): artifact_type_data.preview = '# full text too large to store, storing first {}kb\n{}'.format( self.max_preview_size_bytes//1024, artifact_object[:self.max_preview_size_bytes] ) - if artifact_object: - delete_after_upload = True - override_filename_ext_in_uri = '.txt' - override_filename_in_uri = name + override_filename_ext_in_uri - fd, local_filename = mkstemp(prefix=quote(name, safe="") + '.', suffix=override_filename_ext_in_uri) - os.close(fd) - # noinspection PyBroadException - try: - with open(local_filename, 'wt') as f: - f.write(artifact_object) - except Exception: - # cleanup and raise exception - os.unlink(local_filename) - raise + delete_after_upload = True + override_filename_ext_in_uri = ".txt" + override_filename_in_uri = name + override_filename_ext_in_uri + fd, local_filename = mkstemp(prefix=quote(name, safe="") + ".", suffix=override_filename_ext_in_uri) + os.close(fd) + # noinspection PyBroadException + try: + with open(local_filename, "wt") as f: + f.write(artifact_object) + except Exception: + # cleanup and raise exception + os.unlink(local_filename) + raise elif artifact_object is None or (isinstance(artifact_object, str) and artifact_object == ""): artifact_type = '' - store_as_pickle = False + store_as_pickle = True elif auto_pickle: # revert to pickling the object store_as_pickle = True diff --git a/clearml/storage/helper.py b/clearml/storage/helper.py index e2fb9d4a..f889882f 100644 --- a/clearml/storage/helper.py +++ b/clearml/storage/helper.py @@ -369,13 +369,6 @@ class StorageHelper(object): self._container = self._driver.get_container(container_name=self._base_url) else: # elif self._scheme == 'file': # if this is not a known scheme assume local file - - # If the scheme is file, use only the path segment, If not, use the entire URL - if self._scheme == "file": - url = parsed.path - - url = url.replace("\\", "/") - # url2pathname is specifically intended to operate on (urlparse result).path # and returns a cross-platform compatible result url = parsed.path