-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[New Sample] LlamaIndex Workflows Sample Integration #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[New Sample] LlamaIndex Workflows Sample Integration #179
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
rajeshvelicheti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the contribution, excited to see the LlamaIndex sample.
LGTM overall, but a minor change that might help with maintaining the port numbers overall.
|
Also the CLA check is failing that prevents the PR merge. |
|
@kthota-g I just added myself to the CLA -- hopefully the test passes on the next run |
rajeshvelicheti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@logan-markewich thanks for making the port changes. I was quickly testing this with the cli and it looks great. However, the demo ui (demo/ui) will be broken. It does not support files yet. Is it something that you will be able to address?
|
@rajeshvelicheti ah I completely missed that there was a UI lol Let me see what is required to support this... |
|
@rajeshvelicheti oh boy, this is quite complicated lol but basically, the UI runs an agent, with other agents as tools So we need to
Going to see how hard this is to do now.. I might suggest filling in the README for the UI in at some point as well, took me a while to figure out what this UI was doing π |
Yeah you are right, the UI runs the agent, there is also a host agent. Support file input in the UI, the artifacts service could be your friend here. But there is some bit of code that might still be required. demo/ui/service/server/adk_host_manager.py I agree, it is not very straightforward, please let me know if you have any questions, we can find some time that works for both of us. Alternatively, we just update the Readme suggesting the agents works with cli and UI support might be coming later. |
|
@rajeshvelicheti Yea I don't think I have quite enough time to learn how the UI connects together and I don't want to hold up this PR -- added a note in the readme for this example. We can make the UI support multimodal in another PR another day :) (this is something that would be useful to any agent/example, not just this one!) Also resolved merge conflicts from SK changes, should be good to merge |
|
Thanks for the contribution @logan-markewich. Great to see lllamaindex sample. |
|
https://github.com/google/A2A/issues/197 to track the UI limitation. |
* add llama-index example * update CLI to accept file attachments * update readmes * remaining nits * change default port * update supported content types * readme note
This PR does a few things
Let me know if there is anything I can improve here! The task manager was a little complicated to figure out, I mostly adapted what the langgraph example had.