[fix] Validate interface variable names are python-valid

[ddl-rliu]

Tracking issue

flyteorg/flyte#5511

Why are the changes needed?

Rationale: flyte inputs names are "like" python variable names, which should not use special characters like . (see my.value in the repro code

Expected behavior: The workflow should actually fail at compile time at step 1. (e.g. perform extra validation)

What changes were proposed in this pull request?

Workflow should fail at compile time at step 1, when python-invalid variable name is used like "my.value".

How was this patch tested?

Setup process

import logging

from flytekit import ContainerTask, kwtypes, task, workflow

logger = logging.getLogger(__file__)

calculate_ellipse_area_shell = ContainerTask(
    name="ellipse-area-metadata-shell",
    input_data_dir="/var/inputs",
    output_data_dir="/var/outputs",
    inputs=kwtypes(**{"my.value": float, "c": float}),
    outputs=kwtypes(area=float, metadata=str),
    image="ghcr.io/flyteorg/rawcontainers-shell:v2",
    command=[
        "./calculate-ellipse-area.sh"
    ],
)

@task
def report_all_calculated_areas(
    area_shell: float,
    metadata_shell: str,
):
    logger.info(f"shell: area={area_shell}, metadata={metadata_shell}")

@workflow
def wf(a: float, b: float):
    area_shell, metadata_shell = calculate_ellipse_area_shell(**{"my.value": a, "c": b})

    # Report on all results in a single task to simplify comparison
    report_all_calculated_areas(
        area_shell=area_shell,
        metadata_shell=metadata_shell,
    )

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

[ddl-rliu]

Adds validation on input naming in Interface. Note that this is already done on the outputs:

variables = [k for k in outputs.keys()]
...
collections.namedtuple(..., variables) # collections.namedtuple checks
# the variable names are valid python identifiers

So it is sensible to add similar validation to the inputs, which must also by valid python types.

[eapolinario]

Nice. I was about to mention isidentifier. 😄

[ddl-rliu]

The error message is odd here, but it is the standard error message from collections.namedtuple see: https://github.com/flyteorg/flytekit/pull/2538/files#r1657915913 https://github.com/python/cpython/blob/main/Lib/collections/__init__.py#L401-L402

[eapolinario]

@ddl-rliu , can you fix the lint error in CI? You can run make lint from the root to repro it locally.

[ddl-rliu]

@eapolinario done!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.40%. Comparing base (647b071) to head (fd6a5d7).
Report is 3 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (647b071) and HEAD (fd6a5d7). Click for more details.

HEAD has 13 uploads less than BASE

| Flag | BASE (647b071) | HEAD (fd6a5d7) | |------|------|------| ||15|2|

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2538       +/-   ##
===========================================
- Coverage   91.30%   72.40%   -18.91%     
===========================================
  Files          78      182      +104     
  Lines        3968    18576    +14608     
  Branches        0     3659     +3659     
===========================================
+ Hits         3623    13450     +9827     
- Misses        345     4479     +4134     
- Partials        0      647      +647     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.