We just updated from Flyte release `v0.19.3` to ne...
# announcements
a
We just updated from Flyte release
v0.19.3
to new
v1.0.0
and Flyte Admin no longer wants to register workflows like:
Copy code
@flytekit.workflow
def DDPTrainingWorkflow(
    train_datasets: typing.List[str] = [],
    ...
):
    patch_experiment(train_datasets)

@flytekit.task()
def patch_experiment(
    train_datasets: typing.List[str],
)
^^^^ while registering the workflow, this throws:
Copy code
[2022-04-27T15:09:25Z] 	details = "Type mismatch for Parameter train_datasets in default_inputs has type collection_type:<simple:STRING > , expected simple:NONE "
clearly something doesn't like the default
[]
value of
train_datasets
against its declared type
typing.List[str]
. Ideas?
k
cc @Yee @Eduardo Apolinario (eapolinario)
a
Users are using flytekit 0.23.0b6. The error message is in a gRPC response (so it is coming as part of a response from admin and not from flytekit itselft)
FYI this workflow still uses
flyte-cli register-files
to register this workflow (with flytekit 0.23.0b6). Update - I am able to reproduce this in another repo where I have flytekit 0.32.3 and I use the latest flytectl.
@Ketan (kumare3) @Haytham Abuelfutuh this error is too simple, this should have been caught. You guys need to have a larger set of acceptance workflows that get tested automatically before each new release.
h
Ouch... You're absolutely right! Apologies about the breakage.. we will get right on it. This should be as simple as adding an example in flytesnacks/cookbook/core. All of these get automatically run with every release. Mind filing an issue to do that? Cc @Eduardo Apolinario (eapolinario) to keep me honest ^ Cc @Samhita Alla to help add the example for how to use default inputs for workflows and launchplans...
šŸ‘ 1
Have you also upgraded flytekit version? @Alex Bain
a
I do get this on both flytekit
0.23.0b6
and
0.32.3
. I can't easily upgrade to flytekit 1.0.0 yet.
k
@Alex Bain this is indeed embarassing. Let me try it out. We have not changed the compiler logic
a
Is there an easy way I can tell users to redeclare the default value of their workflow parameter, e.g.
Copy code
@flytekit.workflow
def DDPTrainingWorkflow(
    train_datasets: typing.List[str] = ???,  # was previously []
):
k
no
x: typing.List[str] = []
should work
let me figure out, can you give me a few minutes please?
šŸ‘ 1
@Alex Bain / @Yee what seems is that it only fails for List / Collection
e
@Ketan (kumare3), what about non-empty lists as defaults? Does that work?
k
hmm let me check
yes non empty list defaults work
I think I probably can find the issue, its in the typing on client side then?
@Alex Bain did you upgrade the backend too?
@Eduardo Apolinario (eapolinario) it fails for complex types with empty defaults. This is definitely a regression in the compiler
I am looking now
Actually scratch that, this is a regression in admin
@katrinaā€™s hunch is right. The problem is because of the UnionType compiler change. PR here
@Max Smolin can you help us understand the motivation point 2 = https://github.com/maximsmol/flyte/blob/master/rfc/core%20language/sum-types.md#2-motivation. So removing handing for none type defaults is affecting @Alex Bain
Do you think adding it back impacts union in some way? I do not feel it does - but I maybe wrong
@Alex Bain here is the issue - https://github.com/flyteorg/flyte/issues/2424
a
Thanks for the update, I follow the issue
e
@Alex Bain, we just released flyte 1.0.1. Can you update and verify that it fixes this issue on your side?
a
@Eduardo Apolinario (eapolinario) the issue is not solved currently in Flyte. Although a fix was recently made to Propeller, the issue actually appears to users at task / workflow registration time, which means that more fixes will need to be made to flytekit / flyteadmin.
e
@Alex Bain, The Flyte 1.0.1 release contains the necessary changes to fix this, namely this flytepropeller PR and the corresponding flyteadmin PR. In terms of versions, we fixed the issue in flytepropeller v1.1.1 and flyteadmin v1.1.2, both of which were present in the flyte 1.0.1 release. Can you double-check which version of flyteadmin you're running?
a
Ah, I have been watching the flyteadmin PRs and didn't realize that the fix for the default values was embedded in the PR to which you linked. Let me see if I can verify the fix in my local cluster.
šŸ‘ 1
Indeed, Flyte 1.0.1 did re-enable me to declare non-
None
default values for collection types (e.g.
[]
and
{}
). Thanks for following up.
e
Awesome. We will make sure to include tests to cover this feature going forward.
164 Views