Skip to content

Commit

Permalink
fix(sdk): Fixed UI metadata and metrics (#4672)
Browse files Browse the repository at this point in the history
Reverting most of the #2334 which inadvertently broke those artifacts by causing the names to be mangled.

KFP's DSL compiler prepends template names to output names to ensure global uniqueness of *input* names (DSL's ContainerOp does not have concept of inputs, so the inputs are generated during the compilation including input names). But prepending template names to the output names stops the backend from recognizing the mlpipeline-ui-metadata and mlpipeline-metrics artifacts.
  • Loading branch information
Ark-kun authored Oct 26, 2020
1 parent fc6b5d6 commit 80e1d70
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion sdk/python/kfp/compiler/_op_to_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def _op_to_template(op: BaseOp):
processed_op = _process_base_ops(op)

if isinstance(op, dsl.ContainerOp):
output_artifact_paths = OrderedDict()
output_artifact_paths = OrderedDict(op.output_artifact_paths)
# This should have been as easy as output_artifact_paths.update(op.file_outputs), but the _outputs_to_json function changes the output names and we must do the same here, so that the names are the same
output_artifact_paths.update(sorted(((param.full_name, processed_op.file_outputs[param.name]) for param in processed_op.outputs.values()), key=lambda x: x[0]))

Expand Down
6 changes: 3 additions & 3 deletions sdk/python/kfp/dsl/_container_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,9 @@ def _decorated(*args, **kwargs):
# only proxy public callables
setattr(self, attr_to_proxy, _proxy(attr_to_proxy))

if output_artifact_paths:
warnings.warn('The output_artifact_paths parameter is deprecated since SDK v0.1.32. Use the file_outputs parameter instead. file_outputs now supports outputting big data.', DeprecationWarning)

# Special handling for the mlpipeline-ui-metadata and mlpipeline-metrics outputs that should always be saved as artifacts
# TODO: Remove when outputs are always saved as artifacts
for output_name, path in dict(file_outputs).items():
Expand All @@ -1103,9 +1106,6 @@ def _decorated(*args, **kwargs):
self.artifact_arguments = artifact_arguments
self.file_outputs = file_outputs
self.output_artifact_paths = output_artifact_paths or {}
if output_artifact_paths:
file_outputs.update(output_artifact_paths)
warnings.warn('The output_artifact_paths parameter is deprecated since SDK v0.1.32. Use the file_outputs parameter instead. file_outputs now supports outputting big data.', DeprecationWarning)

self._metadata = None
self._parameter_arguments = None
Expand Down

0 comments on commit 80e1d70

Please sign in to comment.