kaxil commented on code in PR #74694:
URL: https://github.com/apache/airflow/pull/74694#discussion_r4340285181
##########
providers/common/ai/docs/changelog.rst:
##########
@@ -25,6 +25,14 @@
Changelog
---------
+.. note::
+ ``airflow.providers.common.ai.toolsets.logging.LoggingToolset`` (public
since 0.1.0) is
Review Comment:
1.0.0 also shipped the package-level export from #74449 (the Features entry
below), so `from airflow.providers.common.ai.toolsets import LoggingToolset`
breaks too, and that's the import the 1.0.0 toolsets page showed. Could the
note name both paths? On the rebase it needs to stay above main's new `1.1.0`
section, since 1.1.0rc1 still ships the class.
##########
providers/common/ai/docs/stability.rst:
##########
@@ -102,7 +102,7 @@ Pydantic AI toolsets, but the Pydantic AI class they
inherit from can change.
types, a write nested inside the statement fails it too. A reviewer's
edit under
``allow_modifications=True`` is checked the same way. ``db_conn_id``
with
``table_names``, ``schema_context`` or ``datasource_config`` give the
model the
- schema.
+ schema. ``datasource_config`` is experimental; see below.
Review Comment:
1.0.0 shipped `datasource_config` and `data_sources` in these stable rows,
and the top of this page says a stable feature keeps its public parameters
until the next major release, so moving them to experimental in 1.x is what
that policy rules out. The `datafusion<52` pin was already there in 1.0.0. I
asked for this before 1.0.0 went final, so that one's on me, but now I'd keep
both stable, drop the two experimental-table rows and the operator notes, and
mention the pin as a dependency caveat.
##########
providers/common/ai/src/airflow/providers/common/ai/toolsets/logging.py:
##########
@@ -42,15 +42,8 @@
@dataclass
-class LoggingToolset(WrapperToolset[Any]):
- """
- Wrap a toolset to log each tool call with timing.
-
- .. note::
-
- Experimental: this can change or be removed in a minor release of this
provider.
- See :ref:`howto/stability`.
- """
+class _LoggingToolset(WrapperToolset[Any]):
+ """Wrap a toolset to log each tool call with timing; applied through
``ToolLoggingCapability``."""
Review Comment:
The Experimental note removed here didn't move to `ToolLoggingCapability`,
which is now the module's only public class and is listed as experimental in
`stability.rst`. Could its docstring carry the same note as the other
experimental classes?
##########
providers/common/ai/docs/frameworks/index.rst:
##########
@@ -149,10 +149,17 @@ Tested versions
These are the versions the adapter tests have been run against. Strands and
ADK cap
packages below the versions in Airflow's dependency lock, and current OpenAI
Agents SDK
releases need ``openai`` 3, so CI runs their adapter tests in a job of their
own, one
-framework at a time. Pull requests test each framework against the versions in
-Airflow's CI image, which for the OpenAI Agents SDK means the newest release
that accepts
-``openai`` 2.x; scheduled runs test each framework's newest release with its
own
-dependency pins.
+framework at a time.
+
+Outside canary runs, the job holds every package already in the CI image at its
+installed version with ``--override``, so Strands' and ADK's own dependency
caps are
+overridden. It installs the newest framework release that is older than the
repository's
+``exclude-newer`` window, which for the OpenAI Agents SDK means the newest
release that
+accepts ``openai`` 2.x, not the versions in the table above. It therefore does
not show
+that those versions work in the environment a user gets. Canary runs, which
include
+scheduled, push and manual runs on the Apache Airflow repository and pull
requests
Review Comment:
Push runs don't get the framework pins: `ci-amd.yml` has no push trigger for
`main`, and on a `v*-test` push `run_agent_framework_tests` returns False
because providers release from main only. That leaves scheduled and manual runs
on main and PRs labeled `canary`.
--
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]
