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

Introduces separate runtime provider schema #13488

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jan 5, 2021

Based on ##13556

The provider.yaml contains more information that required at
runtime (specifically about documentation building). Those
fields are not needed at runtime and their presence is optional.
Also the runtime check for provider information should be more
relexed and allow for future compatibility (with
additional properties set to false). This way we can add new,
optional fields to provider.yaml without worrying about breaking
future-compatibility of providers with future airflow versions.

This changei restores 'additionalProperties': false in the
main, development-focused provider.yaml schema and introduced
new runtime schema that is used to verify the provider info when
providers are discovered by airflow.

This 'runtime' version should change very rarely as change to
add a new required property in it breaks compatibility of
providers with already released versions of Airflow.

We also trim-down the provider.yaml file when preparing provider
packages to only contain those fields that are required in the
runtime schema.


^ 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, 2021

cc: @mik-laj - for some reason I cannot add you as reviewer?

MANIFEST.in Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch 7 times, most recently from ec40e72 to 426d584 Compare January 5, 2021 15:44
@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch from 426d584 to e34db89 Compare January 6, 2021 10:19
airflow/providers_manager.py Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch 3 times, most recently from ed2a7ba to ab48fc5 Compare January 7, 2021 10:41
@potiuk
Copy link
Member Author

potiuk commented Jan 7, 2021

All comments addressed. I also added more colors in the output with 'rich' to see more clearly errors/OK status.

@github-actions
Copy link

github-actions bot commented Jan 7, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch 2 times, most recently from 973bffb to 904f712 Compare January 7, 2021 22:12
@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch from 904f712 to 7a9e590 Compare January 7, 2021 23:55
@potiuk
Copy link
Member Author

potiuk commented Jan 8, 2021

Looks like it's going to be green (except quarantined build) looking forward to approval!

@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch 2 times, most recently from 152fb49 to 30088c8 Compare January 8, 2021 12:49
@potiuk
Copy link
Member Author

potiuk commented Jan 8, 2021

@kaxil @ashb -> ready to go :)

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jan 8, 2021
@github-actions
Copy link

github-actions bot commented Jan 8, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot removed the full tests needed We need to run full set of tests for this PR to merge label Jan 8, 2021
@github-actions
Copy link

github-actions bot commented Jan 9, 2021

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch from d371eab to e8e6ac4 Compare January 9, 2021 17:12
@github-actions
Copy link

github-actions bot commented Jan 9, 2021

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@github-actions
Copy link

github-actions bot commented Jan 9, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch from e8e6ac4 to 057c397 Compare January 10, 2021 12:04
@potiuk
Copy link
Member Author

potiuk commented Jan 10, 2021

@kaxil -> all fixed and only quarantined tests failing

The provider.yaml contains more information that required at
runtime (specifically about documentation building). Those
fields are not needed at runtime and their presence is optional.
Also the runtime check for provider information should be more
relexed and allow for future compatibility (with
additional properties set to false). This way we can add new,
optional fields to provider.yaml without worrying about breaking
future-compatibility of providers with future airflow versions.

This changei restores 'additionalProperties': false in the
main, development-focused provider.yaml schema and introduced
new runtime schema that is used to verify the provider info when
providers are discovered by airflow.

This 'runtime' version should change very rarely as change to
add a new required property in it breaks compatibility of
providers with already released versions of Airflow.

We also trim-down the provider.yaml file when preparing provider
packages to only contain those fields that are required in the
runtime schema.
@potiuk potiuk force-pushed the add-runtime-version-of-provider-yaml-schema branch from 057c397 to fed91c6 Compare January 11, 2021 10:35
@potiuk potiuk requested a review from kaxil January 11, 2021 11:32
@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jan 11, 2021
@potiuk potiuk merged commit ad2a030 into apache:master Jan 11, 2021
@potiuk potiuk deleted the add-runtime-version-of-provider-yaml-schema branch January 11, 2021 22:10
kaxil pushed a commit that referenced this pull request Jan 21, 2021
The provider.yaml contains more information that required at
runtime (specifically about documentation building). Those
fields are not needed at runtime and their presence is optional.
Also the runtime check for provider information should be more
relexed and allow for future compatibility (with
additional properties set to false). This way we can add new,
optional fields to provider.yaml without worrying about breaking
future-compatibility of providers with future airflow versions.

This changei restores 'additionalProperties': false in the
main, development-focused provider.yaml schema and introduced
new runtime schema that is used to verify the provider info when
providers are discovered by airflow.

This 'runtime' version should change very rarely as change to
add a new required property in it breaks compatibility of
providers with already released versions of Airflow.

We also trim-down the provider.yaml file when preparing provider
packages to only contain those fields that are required in the
runtime schema.

(cherry picked from commit ad2a030)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools full tests needed We need to run full set of tests for this PR to merge kind:documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants