kaxil commented on code in PR #72939: URL: https://github.com/apache/airflow/pull/72939#discussion_r4051148754
########## providers/common/ai/tests/unit/common/ai/test_provider_metadata.py: ########## @@ -0,0 +1,355 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Drift tripwires between ``provider.yaml``'s declared ``external-services`` and what +pydantic-ai actually installs. + +See `files/common-ai-supported-services/label-sources.md` (planning artefact, not shipped) Review Comment: This file doesn't seem to exist anywhere in the repo. It's cited twice, here and again near DEPRECATED_UPSTREAM_MODULES/gateway, as the source backing why gateway isn't excluded even though it's not in python-modules. Since it's not shipped, that justification isn't checkable by anyone reading this later. Might be worth folding the one or two load-bearing facts inline instead of pointing at a file nobody has. ########## devel-common/src/sphinx_exts/providers_extensions.py: ########## @@ -515,8 +514,93 @@ def render_content(self, *, tags: set[str] | None, header_separator: str = DEFAU return _render_openlineage_supported_classes_content() +def _find_provider_package_data(package_name: str) -> dict[str, Any]: + for provider in load_package_data(): + if provider["package-name"] == package_name: + return provider + raise ValueError(f"No provider.yaml found for package '{package_name}'") + + +def _render_connection_services_content(package_name: str) -> str: + provider = _find_provider_package_data(package_name) + rows = [ + { + "hook_name": conn["hook-name"], + "services": conn.get("external-services") or [], + "ref": f"howto/connection:{conn['connection-type']}", + } + for conn in provider.get("connection-types", []) + ] + return _render_template("provider_connection_services.rst.jinja2", rows=rows) + + +# Display name for each toolset module: the class name for modules that map to one +# (`toolsets.rst` documents each under a `` ``ClassName`` `` heading), or a short +# descriptive name for the two that don't (`managed_agent` documents a family of +# provider-specific subclasses under "Managed Agent Toolsets"; `langchain_bridge` is a +# function, documented under "Working with LangChain"). Kept here rather than in +# `provider.yaml` because it is presentation-only, not metadata the registry also needs. +# +# Deliberately no `.get(basename, default)` fallback below: every module in +# `python-modules` must have an entry here, the same way `LABELS` and the anchor check +# in `test_provider_metadata.py` require full coverage. A `KeyError` on a new toolset +# module is the intended failure mode, not a silently rendered basename. +_TOOLSET_DISPLAY_NAMES = { + "hook": "HookToolset", + "sql": "SQLToolset", + "datafusion": "DataFusionToolset", + "logging": "LoggingToolset", + "mcp": "MCPToolset", + "skills": "AgentSkillsToolset", + "sandbox": "SandboxToolset", + "langchain_bridge": "LangChain Bridge", + "managed_agent": "Managed Agent Toolsets", +} + + +def _render_toolset_services_content(package_name: str) -> str: + provider = _find_provider_package_data(package_name) + rows = [] + for toolset in provider.get("toolsets", []): + services_by_module = { + entry["module"]: entry["services"] for entry in toolset.get("external-services") or [] + } + for module in toolset.get("python-modules", []): + basename = module.rsplit(".", 1)[-1] + rows.append( + { + "display_name": _TOOLSET_DISPLAY_NAMES[basename], Review Comment: The comment above explains why a missing key here should fail loudly for a new common.ai toolset module, which makes sense. But `provider-toolset-services` is registered globally in `setup()`, not scoped to common.ai, so the first other provider that adds a toolsets block and reuses this directive hits the same KeyError on its own module basenames, not because it forgot an entry in its own dict but because this dict only ever contained common.ai's names. Might be worth deriving the display name from provider.yaml (e.g. integration-name) instead, or catching KeyError and re-raising with a message naming the provider and module, similar to what `_find_provider_package_data` already does for an unknown package. ########## contributing-docs/23_provider_hook_migration_to_yaml.rst: ########## @@ -109,6 +109,28 @@ supported field options, see type: string default: "my-project" +Toolset metadata is defined under the separate top-level ``toolsets`` key: + +toolsets.external-services +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Per-module list of external services each toolset reaches, added as a sibling of +``python-modules`` inside a ``toolsets`` entry. Like the connection-type +``external-services`` key above, this is intended to be surfaced as a table on the Review Comment: This says the toolsets external-services list is intended to reach the registry the same way connection-type external-services does, but I don't see that wired up anywhere. `ProviderContract` in `dev/registry/registry_contract_models.py` has no `toolsets` field, and `toolset` doesn't show up anywhere in `dev/registry/` or `registry/src/_data/`. The connection-type version actually flows through `extract_metadata.py` and `extract_versions.py`; this one currently only reaches the new Sphinx page. Might be worth softening this to say it's currently surfaced on the docs page, unless there's a follow-up planned to thread it through the registry too. -- 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]
