Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jan 28, 2022

When there are import errors in providers we printed errors
in a folded group which lead to poor discovery of those errors.
Also with recent changes to Airflow main for upcoming 2.3 version
some errors might become more common when developing providers.
Specifically the way how to import Context in order to satisfy
MyPy and keep Airflow 2.1 compatibility is not obvious.

This change introduces helpful guideline to users adding new
providers:

  • moving errors outside of the folded group with imports
  • adding comment explaining what the errors are about
  • adding message about backwards compatibility in case
    errors happen during 2.1.0 backwards-compatibility check
  • adding explanation and suggest a fix in the common Context
    impport error

Just last commit matters!


^ 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 28, 2022

cc: @pateash @kazanzhy

@potiuk potiuk force-pushed the better-errors-when-importing-providers-has-errors branch 5 times, most recently from fe3751c to 82aac91 Compare January 29, 2022 18:05
@potiuk potiuk closed this Jan 29, 2022
@potiuk potiuk reopened this Jan 29, 2022
@potiuk potiuk force-pushed the better-errors-when-importing-providers-has-errors branch 3 times, most recently from 902e6d0 to eb3054f Compare January 29, 2022 21:23
@potiuk potiuk marked this pull request as ready for review January 29, 2022 21:24
@potiuk
Copy link
Member Author

potiuk commented Jan 29, 2022

Much better errror output:

Screenshot 2022-01-29 at 22 23 01

@potiuk potiuk force-pushed the better-errors-when-importing-providers-has-errors branch from eb3054f to cb54e76 Compare January 29, 2022 21:30
@potiuk potiuk requested a review from turbaszek as a code owner January 29, 2022 21:31
@potiuk potiuk force-pushed the better-errors-when-importing-providers-has-errors branch 3 times, most recently from ed59b16 to c30dd2a Compare January 30, 2022 16:37
@potiuk
Copy link
Member Author

potiuk commented Jan 30, 2022

Right - this is the final version:

Screenshot from 2022-01-30 17-35-59

I just pushed it error-free so that we can merge it.

When there are import errors in providers we printed errors
in a folded group which lead to poor discovery of those errors.
Also with recent changes to Airflow main for upcoming 2.3 version
some errors might become more common when developing providers.
Specifically the way how to import Context in order to satisfy
MyPy and keep Airflow 2.1 compatibility is not obvious.

This change introduces helpful guideline to users adding new
providers:

* moving errors outside of the folded group with imports
* adding comment explaining what the errors are about
* adding message about backwards compatibility in case
  errors happen during 2.1.0 backwards-compatibility check
* adding explanation and suggest a fix in the common Context
  impport error
@potiuk potiuk force-pushed the better-errors-when-importing-providers-has-errors branch from c30dd2a to 797794c Compare January 30, 2022 16:58
@potiuk potiuk requested a review from eladkal January 31, 2022 22:51
@potiuk
Copy link
Member Author

potiuk commented Feb 1, 2022

This one will be nice for users who struggle with Airflow 2.1 compatibility for providers :).

@potiuk
Copy link
Member Author

potiuk commented Feb 1, 2022

cc: @pateash WDYT ?

Copy link
Contributor

@pateash pateash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@pateash
Copy link
Contributor

pateash commented Feb 1, 2022

cc: @pateash WDYT ?

This looks great. Cheers.

@potiuk
Copy link
Member Author

potiuk commented Feb 6, 2022

Anyone please :) . This will be a life-saver for those who will be adding features for providers. @turbaszek I think you even requested it once and I thought it was fixed - but this one should be way better (see #13924)

@potiuk potiuk requested a review from uranusjr February 6, 2022 16:56
@potiuk
Copy link
Member Author

potiuk commented Feb 8, 2022

Hello :) :) :)

@potiuk
Copy link
Member Author

potiuk commented Feb 8, 2022

Just copying the nice image again to drag attention:

image

Pretty please:

image

@potiuk potiuk merged commit 13b6a57 into apache:main Feb 8, 2022
@potiuk potiuk deleted the better-errors-when-importing-providers-has-errors branch February 8, 2022 22:06
@potiuk potiuk restored the better-errors-when-importing-providers-has-errors branch April 26, 2022 20:49
@potiuk potiuk deleted the better-errors-when-importing-providers-has-errors branch July 29, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants