unexpected parameter name conversion by DSL

[IronPan]

Here is snippet of the pipeline definition

@dsl.pipeline(
  name='training',
  description='Example training pipeline'
)
def foo(project, working_dir):
....

And when I submit a run I need to do

run = client.run_pipeline(exp.id, 'training', 'training.tar.gz',
                          params={'project': PROJECT, 'working-dir': WORKING_DIR})

Here working_dir is converted to working-dir which is somewhat unexpected. I have to look into the generated yaml to find out what it actual converted to.

[Ark-kun]

Here is snippet of the pipeline definition

Do we really have a snippet like that? AFAIK, right now all pipeline parameters look like this: predict_mode: dsl.PipelineParam=dsl.PipelineParam(name='predict-mode', value='local')

I wonder what happens with the parameter names in https://github.com/kubeflow/pipelines/pull/110/files

I agree that this needs to be handled in a way that's easy for the user. Quite a good chunk of my python component support code deals with naming.

[IronPan]

I think we already support raw parameter.
in https://github.com/kubeflow/pipelines/blob/master/samples/notebooks/KubeFlow%20Pipeline%20Using%20TFX%20OSS%20Components.ipynb

@dsl.pipeline(
  name='TFX Taxi Cab Classification Pipeline Example',
  description='Example pipeline that does classification with model analysis based on a public BigQuery dataset.'
)
def my_taxi_cab_classification(
    output,
    project,
    model,
    column_names=dsl.PipelineParam(......

[Ark-kun]

I confirm that this issue still exists (invalid spec: spec.arguments.param-1.value is required during execution). @gaoning777 , can you look at the pipeline parameter sanitization? Argo now allows underscores in parameter names.

The actual regexp is [-a-zA-Z0-9_]+.

Update: Argo has not released the underscore fix yet =( argoproj/argo-workflows#1048

[gaoning777]

The naming sanitization is necessary to convert the python names to argo compatible names.
The next argo release should support underscores and we do not need the underscore to dash conversion.

[gaoning777]

@Ark-kun is argo supporting the underscore in 2.3, the current version pipeline deploys?

[gaoning777]

@Ark-kun gently tap.

[Ark-kun]

@Ark-kun is argo supporting the underscore in 2.3, the current version pipeline deploys?

/cc @gaoning777
Yes. We're deploying Argo 2.3 since June 1
kubeflow/kubeflow#3349 kubeflow/kubeflow#3374 https://github.com/kubeflow/kubeflow/commits/pipelines/kubeflow

Unfortunately, many of our customers probably have older versions. Their experience will be broken if they try to run pipelines with inputs/outputs containing underscores.
Do you think it's best to make the change right now or should we wait more, so that the users can upgrade?

[Ark-kun]

Fixed by #466