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

Fix main failing on wrong inheritance hierarchy #20687

Closed
wants to merge 1 commit into from

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jan 5, 2022

Introduced by ##20332


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

Failed with

tests/core/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_256_airflow_providers_amazon_aws_operators_ecs_ECSOperator: TypeError: Protocols can only inherit from other protocols, got <class 'airflow.providers.amazon.aws.operators.ecs.ECSProtocol'>

tests/core/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_256_airflow_providers_amazon_aws_operators_ecs_ECSOperator: TypeError: Protocols can only inherit from other protocols, got <class 'airflow.providers.amazon.aws.operators.ecs.ECSProtocol'>

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

No-one fault - selective checks did not run the tests as it was "provider-only" change (theorethically).

@ashb
Copy link
Member

ashb commented Jan 5, 2022

But ECSProtocol is a protocol...

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

But ECSProtocol is a protocol...

I know . But It helped for now. I tihnk there is some problem with the type check. Since it is a deprecated class, I preferred to quickly fix it for now.

@uranusjr
Copy link
Member

uranusjr commented Jan 5, 2022

So is EcsProtocol. By inheriting from EcsProtocol, ECSProtocol is already automatically a protocol, and inheriting from Protocol is redundant either way.

@uranusjr
Copy link
Member

uranusjr commented Jan 5, 2022

See also #20669 and #20670.

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

        for base in cls.__bases__:                                                                                                                                                                                                                                                                               
            if not (base in (object, Generic) or                                                                                                                                                                                                                                                                 
                    base.__module__ == 'collections.abc' and                                                                                                                                                                                                                                                     
                    base.__name__ in _PROTO_WHITELIST or                                                                                                                                                                                                                                                         
                    isinstance(base, _ProtocolMeta) and base._is_protocol):                                                                                                                                                                                                                                      
                raise TypeError('Protocols can only inherit from other'                                                                                                                                                                                                                                          
>                               ' protocols, got %r' % base)                                                                                                                                                                                                                                                     
E               TypeError: Protocols can only inherit from other protocols, got <class 'airf

@uranusjr
Copy link
Member

uranusjr commented Jan 5, 2022

I’m going to close this one since this is only a subset of #20670, and we should choose from either it or #20669 instead.

@uranusjr uranusjr closed this Jan 5, 2022
@eladkal
Copy link
Contributor

eladkal commented Jan 5, 2022

No-one fault - selective checks did not run the tests as it was "provider-only" change (theorethically).

yep #20332 had a green build.
We need test_core_to_contrib to run also on any change in providers.
I think also this file name is a bit misleading.. it has also entries for stuff other than contrib. it's more like a general deprecation check.

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

No-one fault - selective checks did not run the tests as it was "provider-only" change (theorethically).

yep #20332 had a green build. We need test_core_to_contrib to run also on any change in providers. I think also this file name is a bit misleading.. it has also entries for stuff other than contrib. it's more like a general deprecation check.

Agree - we even have a special category for that "Always" :)

@potiuk
Copy link
Member Author

potiuk commented Jan 5, 2022

PR in #20691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants