fix(runner): default InMemoryRunner appName to match Python SDK - #1485
mithun-sudo wants to merge 1 commit into
Conversation
|
Hi @mithun-sudo, thank you for your contribution and We appreciate you taking the time to submit this pull request. I have noticed that the maven build is failing with your changes, could you please address this? |
Default the single-arg InMemoryRunner constructor to "InMemoryRunner" instead of agent.name(), aligning with adk-python.
405766a to
d4cafa4
Compare
|
Fixed CI: spring-ai tests were still creating sessions with agent.name() while InMemoryRunner now defaults to "InMemoryRunner". Updated session creation to use runner.appName(). ./mvnw -Prelease clean package passes locally. |
|
@mithun-sudo, thank you for addressing the comments. This PR is currently under review by our team and we will keep you posted if any further information is required. |
|
Hi @mithun-sudo, thank you for the contribution and for addressing the TODO! Unfortunately, we cannot merge this change. Changing the default When callers create a session ahead of time via:
Anyone who specifically needs appName to be "InMemoryRunner" (for example, for cross-language parity) can achieve it that way today without breaking existing callers. In this case, backwards compatibility for existing Java users is more important than parity with Python. That TODO comment shouldn't have been left in the codebase; we will take care of cleaning it up on our end. Thank you again for taking the time to send the PR, and sorry for the churn caused by that comment! |
Fixes #1486
Summary
InMemoryRunner(BaseAgent)appName to"InMemoryRunner"instead ofagent.name(), matching adk-python.InMemoryRunnerTestand updateAgentWithMemoryTestto userunner.appName().Breaking change (minor)
Callers that relied on implicit
agent.name()as appName should usenew InMemoryRunner(agent, agent.name())explicitly.Test plan
./mvnw -pl core -am test— BUILD SUCCESS