-
Notifications
You must be signed in to change notification settings - Fork 69
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
Update the email me a poem sample #678
Conversation
IMO the mail server component is quite important for demoing (I've shown this at several conferences now)... |
What I really don't like is that unless I actually do a prerequisite step, the sample doesn't work - I really want demoers to have not to do anything but start the application to get the basic functionality working |
Could we then switch to https://github.com/quarkiverse/quarkus-mailpit? I haven't tried that, but it claims it can run an SMTP server as a dev service, and the emails can be viewed in the Dev UI, that actually sounds cool |
Oh, that's nice! Let me try it and see how it goes |
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.
Good changes Overall but should we make the telemetry optional as it's quite heavy to fetch and it's optional for showing the main funktionality of tools calling. ?
We can do that |
@jmartisk the Mailpit extension is amazing, I'm adding it |
+1 for mailpit. I'm actually wondering if we shouldn't simply make the devservice that comes by default with mailer. But that's a separate issue. |
Yeah, mailpit could be a very nice default |
* Use quarkus-mailpit - This is done to make the setup simpler * Introduce the LGTM observability Dev Service - This is done so users don't have to start anything manually - For this to work, users need to use -Dobservability * Use gpt-4o - This is done because GTP-3.x can sometimes result in multiple tool invocations - Improve the prompt to ensure that no markers are added * Update README accordingly
PR updated |
@jmartisk I temporarily disabled |
Yeah I'm looking at it but I'm puzzled, because it just started failing independently of any changes in our codebase (and the Chroma image version didn't get updated either) |
The following changes have been made: