Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlyteABCMeta Instancecheck Is Wrong #522

Closed
2 of 20 tasks
gfee-lyft opened this issue Sep 24, 2020 · 3 comments
Closed
2 of 20 tasks

FlyteABCMeta Instancecheck Is Wrong #522

gfee-lyft opened this issue Sep 24, 2020 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@gfee-lyft
Copy link

Describe the bug
I found this:

16:19:28 File "/code/gschemavalidator/workflows/tasks.py", line 56, in
16:19:28 @python_task()
16:19:28 File "/srv/service/venv/lib/python3.6/site-packages/flytekit/sdk/tasks.py", line 103, in apply_outputs_wrapper
16:19:28 _type_helpers.python_std_to_sdk_type(v).to_flyte_literal_type(),
16:19:28 File "/srv/service/venv/lib/python3.6/site-packages/flytekit/common/types/helpers.py", line 54, in python_std_to_sdk_type
16:19:28 out = e.python_std_to_sdk_type(t)
16:19:28 File "/srv/service/venv/lib/python3.6/site-packages/flytekit/type_engines/default/flyte.py", line 69, in python_std_to_sdk_type
16:19:28 elif isinstance(t, _base_sdk_types.FlyteSdkType):
16:19:28 File "/srv/service/venv/lib/python3.6/site-packages/flytekit/models/common.py", line 15, in instancecheck
16:19:28 return super(FlyteABCMeta, cls).instancecheck(instance)
16:19:28 TypeError: instancecheck() missing 1 required positional argument: 'instance'
16:19:28 Makefile:51: recipe for target 'register_for_pr' failed

When I created a workflow with a task like, note that should be Types.Schema():

@outputs(thing=Types.Schema)
@python_task()
def my_task(wf_params, thing):
pass

Expected behavior
This should fail with a clear error message

Flyte component

  • Overall
  • Flyte Setup and Installation scripts
  • Flyte Documentation
  • Flyte communication (slack/email etc)
  • FlytePropeller
  • FlyteIDL (Flyte specification language)
  • Flytekit (Python SDK)
  • FlyteAdmin (Control Plane service)
  • FlytePlugins
  • DataCatalog
  • FlyteStdlib (common libraries)
  • FlyteConsole (UI)
  • Other

To Reproduce
Steps to reproduce the behavior:

  1. Make a task like above
  2. Try to register

Screenshots
If applicable, add screenshots to help explain your problem.

Environment
Flyte component

  • Sandbox (local or on one machine)
  • Cloud hosted
    • AWS
    • GCP
    • Azure
  • Baremetal
  • Other

Additional context
Add any other context about the problem here.

@gfee-lyft gfee-lyft added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Sep 24, 2020
@wild-endeavor
Copy link
Contributor

The error message should be clearer yes, but I'll deal with it after the mypy flytekit is done. We're also gonna get rid of six so maybe the base abc can just go away too in favor of the default one. And you know it should be Types.Schema() right?

@kumare3 kumare3 added this to the 0.9.0 milestone Sep 30, 2020
@EngHabu EngHabu modified the milestones: 0.9.0, 0.10.0 Nov 4, 2020
@EngHabu EngHabu removed this from the 0.10.0 milestone Jan 11, 2021
@kumare3 kumare3 removed the untriaged This issues has not yet been looked at by the Maintainers label Jan 29, 2021
@kumare3
Copy link
Contributor

kumare3 commented Feb 28, 2021

@wild-endeavor Maybe we should clear up this issue, if we think with 3.7+ this is not a problem

@kumare3 kumare3 added this to the 0.12.0 milestone Feb 28, 2021
@kumare3
Copy link
Contributor

kumare3 commented Mar 19, 2021

Closing this bugt, as we did not hear back and a fix has been merged

@kumare3 kumare3 closed this as completed Mar 19, 2021
eapolinario added a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
Signed-off-by: Eduardo Apolinario <[email protected]>

Co-authored-by: Eduardo Apolinario <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Jul 24, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants