Repository: Netflix/metaflow
Pull request: handling exceptions for non-picklizable artifacts
This resolves  #763 

Problem statement:
Better error message on failure of Pickle serialization
When Metaflow serializes artifacts to Pickle, It will be a good practice to fail loudly by stating what artifact couldn't be serialized. Currently, when Metaflow fails pickle serialization, it doesn't tell me which particular artifact was the cause of failure during pickle serialization.

Hints from issue discussion:

Commit message: Nit fixes.

Patch to review:
diff --git a/metaflow/datastore/exceptions.py b/metaflow/datastore/exceptions.py
index 139a2c25f43..3b77b21cbb8 100644
--- a/metaflow/datastore/exceptions.py
+++ b/metaflow/datastore/exceptions.py
@@ -1,4 +1,12 @@
 from ..exception import MetaflowException
 
 class DataException(MetaflowException):
-    headline = "Data store error"
\ No newline at end of file
+    headline = "Data store error"
+
+
+class UnpicklizableArtifactException(MetaflowException):
+    headline = "Cannot pickle artifact"
+
+    def __init__(self,artifact_name):
+        msg = 'Cannot pickle dump artifact named "%s"' % artifact_name
+        super().__init__(msg=msg, lineno=None)
\ No newline at end of file
diff --git a/metaflow/datastore/task_datastore.py b/metaflow/datastore/task_datastore.py
index f2d352f099b..25bf1111965 100644
--- a/metaflow/datastore/task_datastore.py
+++ b/metaflow/datastore/task_datastore.py
@@ -13,7 +13,7 @@
 from ..parameters import Parameter
 from ..util import Path, is_stringish, to_fileobj
 
-from .exceptions import DataException
+from .exceptions import DataException, UnpicklizableArtifactException
 
 def only_if_not_done(f):
     @wraps(f)
@@ -234,7 +234,10 @@ def pickle_iter():
                         raise DataException(
                             "Artifact *%s* requires a serialization encoding that "
                             "requires Python 3.4 or newer." % name)
-                    blob = pickle.dumps(obj, protocol=4)
+                    try:
+                        blob = pickle.dumps(obj, protocol=4)
+                    except TypeError as e: 
+                        raise UnpicklizableArtifactException(name)
                 else:
                     try:
                         blob = pickle.dumps(obj, protocol=2)
@@ -246,7 +249,13 @@ def pickle_iter():
                                 "Artifact *%s* is very large (over 2GB). "
                                 "You need to use Python 3.4 or newer if you want to "
                                 "serialize large objects." % name)
-                        blob = pickle.dumps(obj, protocol=4)
+                        try:
+                            blob = pickle.dumps(obj, protocol=4)
+                        except TypeError as e: 
+                            raise UnpicklizableArtifactException(name)
+                    except TypeError as e: 
+                        raise UnpicklizableArtifactException(name)
+
                 self._info[name] = {
                     'size': len(blob),
                     'type': str(type(obj)),


Code review comments:
diff_hunk: @@ -246,7 +249,13 @@ def pickle_iter():
                                 "Artifact *%s* is very large (over 2GB). "
                                 "You need to use Python 3.4 or newer if you want to "
                                 "serialize large objects." % name)
-                        blob = pickle.dumps(obj, protocol=4)
+                        try:
+                            blob = pickle.dumps(obj, protocol=4)
+                        except TypeError as e: 
+                            raise UnpicklizableArtifactException(name)
path: metaflow/datastore/task_datastore.py
text: @user1:
Minor nit - `UnpicklableArtifactException`