kaxil commented on code in PR #71104:
URL: https://github.com/apache/airflow/pull/71104#discussion_r3720139677
##########
airflow-core/src/airflow/provider_info.schema.json:
##########
@@ -432,38 +432,49 @@
"type": "array",
"description": "Apply custom decorators to the TaskFlow API. Can
be accessed by users via '@task.<name>'",
"items": {
- "name": {
- "type": "string"
+ "type": "object",
+ "properties": {
+ "name": {
+ "description": "Name the decorator is exposed under,
following '@task.'",
+ "type": "string"
+ },
+ "class-name": {
+ "description": "Class name that implements the
decorator",
+ "type": "string"
+ }
},
- "path": {
- "type": "string"
- }
+ "required": [
+ "name",
+ "class-name"
+ ]
}
},
"plugins": {
"type": "array",
"description": "Plugins provided by the provider",
"items": {
- "name": {
- "type": "string",
- "description": "Name of the plugin"
+ "type": "object",
+ "properties": {
+ "name": {
+ "type": "string",
+ "description": "Name of the plugin"
+ },
+ "plugin-class": {
+ "type": "string",
+ "description": "Class to instantiate the plugin"
+ }
},
- "plugin-class": {
- "type": "string",
- "description": "Class to instantiate the plugin"
- }
+ "required": [
+ "name",
+ "plugin-class"
+ ]
}
},
"queues": {
"type": "array",
- "description": "Message Queues exposed by the provider",
+ "description": "Message queue provider class names",
"items": {
- "name": {
- "type": "string"
- },
- "message-queue-class": {
- "type": "string"
- }
+ "type": "string"
Review Comment:
This is the runtime schema, and `discover_all_providers_from_packages`
documents it as deliberately looser than the dev one: "The runtime version is
more relaxed (allows for additional properties) and verifies only the subset of
fields that are needed at runtime".
The
[`provider_schema_validator.validate(provider_info)`](https://github.com/apache/airflow/blob/e69c1881b32c36abb827bae3717eaf46424427bd/shared/providers_discovery/src/airflow_shared/providers_discovery/providers_discovery.py#L325)
call there isn't wrapped, and neither caller wraps it
([`ProvidersManager.initialize_providers_list`](https://github.com/apache/airflow/blob/e69c1881b32c36abb827bae3717eaf46424427bd/airflow-core/src/airflow/providers_manager.py#L524)
and the Task SDK's
[`providers_manager_runtime.py`](https://github.com/apache/airflow/blob/e69c1881b32c36abb827bae3717eaf46424427bd/task-sdk/src/airflow/sdk/providers_manager_runtime.py#L198)).
So an installed third-party provider emitting the old dict shape moves from a
logged warning, where
[`_correctness_check`](https://github.com/apache/airflow/blob/e69c1881b32c36abb827bae3717eaf46424427bd/airflow-core/src/airflow/providers_manager.py#L345)
catches the `import_string` failure and skips just that entry, to a ValidationE
rror that aborts discovery so no provider loads at all in that process,
workers included.
Its queue entry is already broken either way, so this is about the blast
radius: one dead queue entry becomes zero providers. Tightening
`provider.yaml.schema.json` alone catches the authoring mistake without that.
The `required` blocks above are a milder version of the same thing, since a
wrong key there already raises `KeyError` in
`_discover_plugins`/`_discover_taskflow_decorators`, though after this change
it takes out the whole provider list rather than just plugin discovery.
`queues` is the one that goes from survivable to fatal.
##########
airflow-core/src/airflow/provider.yaml.schema.json:
##########
@@ -557,12 +557,21 @@
"type": "array",
"description": "Decorators to use with the TaskFlow API. Can be
accessed by users via '@task.<name>'",
"items": {
- "name": {
- "type": "string"
+ "type": "object",
+ "properties": {
+ "name": {
+ "description": "Name the decorator is exposed under,
following '@task.'",
+ "type": "string"
+ },
+ "class-name": {
+ "description": "Class name that implements the
decorator",
+ "type": "string"
+ }
},
- "path": {
- "type": "string"
- }
+ "required": [
Review Comment:
Worth adding `"additionalProperties": false` here and on the `plugins` block
below, otherwise a stray `path:` sitting next to a correct `class-name:` still
validates, which is the confusion this PR is closing. The top level of this
file already sets it, as do most of the object item blocks above. I checked and
no in-tree `provider.yaml` breaks with it added. Leaving
`provider_info.schema.json` alone is right, since the runtime schema is
documented as allowing extra properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
