kubeflow / pipelines

Machine Learning Pipelines for Kubeflow
https://www.kubeflow.org/docs/components/pipelines/
Apache License 2.0
3.61k stars 1.63k forks source link

Compiler accepts runs with non-string env vars #3286

Closed RunOrVeith closed 4 years ago

RunOrVeith commented 4 years ago

What steps did you take:

Add an environment variable to the container of a ContainerOp, and the value of that variable is not of type string, but e.g. int.

What happened:

The compiler compiles everything fine, but running the pipeline crashes with the following error:

HTTP response body: {"error":"Failed to create a new run.: Failed to fetch workflow spec.: Get pipeline YAML failed.: InternalServerError: Failed to unmarshal pipelines/fb244a34-bc9b-49b2-87ac-684f07c70cf7: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string","message":"Failed to create a new run.: Failed to fetch workflow spec.: Get pipeline YAML failed.: InternalServerError: Failed to unmarshal pipelines/fb244a34-bc9b-49b2-87ac-684f07c70cf7: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string","code":13,"details":[{"@type":"type.googleapis.com/api.Error","error_message":"Internal Server Error","error_details":"Failed to create a new run.: Failed to fetch workflow spec.: Get pipeline YAML failed.: InternalServerError: Failed to unmarshal pipelines/fb244a34-bc9b-49b2-87ac-684f07c70cf7: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field EnvVar.value of type string"}]}

What did you expect to happen:

Either one of two thing:

  1. It works and just passes the environment variable as it is,
  2. The compiler tells me that environment variables must be of type string.

Environment:

How did you deploy Kubeflow Pipelines (KFP)? Full Deployment

KFP version: Build commit ee207f2

KFP SDK version: 0.2.5

Anything else you would like to add:

Code to reproduce:

import tempfile
from pathlib import Path

from kfp import dsl, compiler, Client
from kubernetes.client.models import V1EnvVar  # Make sure you have kubernetes <11.0.0, the latest version is incompatible with kfp

@dsl.pipeline(name="it will compile", description="It won't run due to type errors.")
def create_pipeline():
    op = dsl.ContainerOp(
        name='echo',
        image='library/bash:4.4.23',
        command=['sh', '-c'],
        arguments=['echo', "$SOMETHING"])

    # This will compile just fine, but when starting a run, a Go-side type error will be raised,
    # because the value is not a string
    op.container.add_env_variable(V1EnvVar(name="SOMETHING", value=1))

if __name__ == '__main__':
    client = Client()
    experiment = client.create_experiment(name="example-experiment")
    with tempfile.TemporaryDirectory() as tmp_dir:
        pipeline_file_path = str(Path(tmp_dir) / "example_pipeline.tar.gz")
        compiler.Compiler().compile(pipeline_func=create_pipeline, package_path=pipeline_file_path)
        uploaded = client.upload_pipeline(pipeline_package_path=pipeline_file_path, pipeline_name="example_pipeline")

    run = client.run_pipeline(job_name="Crash with int var", pipeline_id=uploaded.id, experiment_id=experiment.id)
    client.wait_for_run_completion(run_id=run.id, timeout=float("inf"))

/kind bug /area sdk

Bobgy commented 4 years ago

/assign @Ark-kun @numerology

Ark-kun commented 4 years ago

It's a bit non-trivial to detect broken Kubernetes structures since Kubernetes' python client does not have this capability.

Would you like us to fix the env case or the general case?

As a workaround, to enable detection at the compilation time, please have argo executable available in $PATH. Then all compiled pipelines will be linted at compilation time.

RunOrVeith commented 4 years ago

Well, ideally of course the general case would be fixed, but I understand that that is probably not quite in-scope for now. I think having a more descriptive error would help most for now, it was not that simply to figure out what caused the error (my actual use case is much larger than the example above). If you only know that something is not a string that should be a string, it takes some guessing. However, I assume that this would require changes to argo, and is not a kubeflow issue.

Having argo in $PATH works for me in the meantime.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically closed because it has not had recent activity. Please comment "/reopen" to reopen it.