Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 15, 2023

This is mostly for the capacity of adding configuration to provider.yaml files.

This change adds capavilty of adding "config" option to provider yaml and provider_info. This is not yet used, but it has been extracted as separate PR from #32604 in an attempt to make it more reviewable and readable.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

This is mostly for the capacity of adding configuration to
provider.yaml files.

This change adds capavilty of adding "config" option to provider
yaml and provider_info. This is not yet used, but it has been
extracted as separate PR from apache#32604 in an attempt to make it more
reviewable and readable.
Copy link
Member

Choose a reason for hiding this comment

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

This entire file is reindented, likely unintended

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. It had inconsistent indentation to begin with so I re-indented it to be fully consistent. I could separate it out like "indent those provider.json.schemas". but I figured that with such a big addition it makes sense to do it in one PR - especially that it has very little "code" in them - it's merely a description that is used to validate the real code.

Do you think it's better to split it ? I am happy to do it if you think it makes sense @uranusjr :)

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

It looks good! And yes, this completely removes my concerns explained in 32628

LGTJM

@potiuk potiuk merged commit f1e1cdc into apache:main Jul 16, 2023
@potiuk potiuk deleted the add-config-option-to-providers-manager branch July 16, 2023 15:09
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 2, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:new-feature Changelog: New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants