[api][python] Introduce YAML API for declaring agents#670
Conversation
4a2cd9f to
665b593
Compare
|
|
||
| name: str | ||
| description: str | None = None | ||
| namespace: str | None = None |
There was a problem hiding this comment.
The name namespace is confusing. Maybe something like default_func_path. Anyway, the semantic of this needs well description in documents.
There was a problem hiding this comment.
Do we need separate defaults for python and java?
There was a problem hiding this comment.
I think this default needs more careful design, otherwise it may introduce more confusion than convenience. I'd suggest to make this a follow-up and exclude from this PR.
| name: str | ||
| function: str | None = None | ||
| type: Language | None = None | ||
| parameter_types: List[str] | None = None |
There was a problem hiding this comment.
This needs a more detailed description. What are the required formats/values of the parameter type strings.
| """SafeLoader that recognizes only ``true``/``false`` as booleans. | ||
|
|
||
| PyYAML follows YAML 1.1 by default, where ``on``/``off``/``yes``/``no`` | ||
| also coerce to booleans. ``on:`` is the natural way to declare an |
There was a problem hiding this comment.
Can we change to another keyword?
| if "." in function: | ||
| qualified = function |
There was a problem hiding this comment.
What if namespace is foo and the function is foo.bar.Func?
| return agents, shared_resources, shared_actions, agent_specs, doc | ||
|
|
||
|
|
||
| def _assert_has_python_implementation( |
There was a problem hiding this comment.
Is this necessary? What if I implemented an agent in yaml + java, but need to use it in a pyflink job?
|
|
||
| def test_load_yaml_string_ref_to_missing_shared_action_errors() -> None: | ||
| env = AgentsExecutionEnvironment.get_execution_environment() | ||
| bad = _FIXTURES / "bad_missing_shared_action.yaml" |
There was a problem hiding this comment.
Should not write temporal contents in the source directory.
The agent-level ``namespace`` default + ``<namespace>.<name>`` fallback for ``function`` was deferred to a follow-up per reviewer feedback (PR apache#670 comment by xintongsong): the naming was confusing, the python/java default story was unclear, and edge cases like ``namespace=foo`` + ``function=foo.bar.Func`` needed more design. Every action and tool entry now requires an explicit fully-qualified ``function:``. Loader, fixtures, e2e YAML and the checked-in JSON Schema are updated to match; the resolve_function rules collapse to: function given → split at last ``.``; function missing → error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: an agent declared in YAML + Java should be runnable from a PyFlink job. The loader was refusing those plans up front, which prejudges what the runtime can host. Drop the gate (and its two helpers + the corresponding test) and let the runtime own whatever support gaps remain — bugs should surface as runtime errors at the actual boundary, not as a parse-time refusal. A separate cross-language ChatMessage role serdes bug currently keeps Java→Python ChatRequestEvent dispatch from round-tripping through Python actions, but that's an upstream serdes issue and not a reason for the YAML loader to filter plans. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: spell out exactly what strings are accepted in a Java tool's ``parameter_types`` list — order, primitive aliases, fully-qualified reference types (boxed primitives included), and the no-generics rule (JVM method descriptors don't carry type parameters). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: the previous keyword collided with YAML 1.1's boolean resolver (which coerces unquoted 'on' / 'off' / 'yes' / 'no'), forcing a custom SafeLoader subclass to patch the bool resolver. The new keyword 'listen_to:' aligns with the Java @action(listenEventTypes = {...}) annotation and lets us drop the loader workaround so the YAML stays portable to any standard YAML 1.1 / 1.2 consumer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The agent-level ``namespace`` default + ``<namespace>.<name>`` fallback for ``function`` was deferred to a follow-up per reviewer feedback (PR apache#670 comment by xintongsong): the naming was confusing, the python/java default story was unclear, and edge cases like ``namespace=foo`` + ``function=foo.bar.Func`` needed more design. Every action and tool entry now requires an explicit fully-qualified ``function:``. Loader, fixtures, e2e YAML and the checked-in JSON Schema are updated to match; the resolve_function rules collapse to: function given → split at last ``.``; function missing → error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: an agent declared in YAML + Java should be runnable from a PyFlink job. The loader was refusing those plans up front, which prejudges what the runtime can host. Drop the gate (and its two helpers + the corresponding test) and let the runtime own whatever support gaps remain — bugs should surface as runtime errors at the actual boundary, not as a parse-time refusal. A separate cross-language ChatMessage role serdes bug currently keeps Java→Python ChatRequestEvent dispatch from round-tripping through Python actions, but that's an upstream serdes issue and not a reason for the YAML loader to filter plans. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: spell out exactly what strings are accepted in a Java tool's ``parameter_types`` list — order, primitive aliases, fully-qualified reference types (boxed primitives included), and the no-generics rule (JVM method descriptors don't carry type parameters). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR apache#670 review comment by xintongsong: the previous keyword collided with YAML 1.1's boolean resolver (which coerces unquoted 'on' / 'off' / 'yes' / 'no'), forcing a custom SafeLoader subclass to patch the bool resolver. The new keyword 'listen_to:' aligns with the Java @action(listenEventTypes = {...}) annotation and lets us drop the loader workaround so the YAML stays portable to any standard YAML 1.1 / 1.2 consumer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Linked issue: #625
Purpose of change
Tests
ut & e2e
API
Yes, add yaml api.
Documentation
doc-neededdoc-not-neededdoc-included