Skip to content
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

Eliminate BeanXProviderSupplier #43

Closed
cescoffier opened this issue Nov 20, 2023 · 7 comments · Fixed by #51
Closed

Eliminate BeanXProviderSupplier #43

cescoffier opened this issue Nov 20, 2023 · 7 comments · Fixed by #51

Comments

@cescoffier
Copy link
Collaborator

Discovering this value is hard. Adding the implementation class name might be simpler.

@geoand
Copy link
Collaborator

geoand commented Nov 20, 2023

Discovering this value is hard

I agree.

Adding the implementation class name might be simpler.

You mean allowing the user to just add the class name they want to be used?

@cescoffier
Copy link
Collaborator Author

I believe it would be simpler if the user can just write:

@RegisterAiService(retriever=TheClassNameOfMyBean.class)

@geoand
Copy link
Collaborator

geoand commented Nov 21, 2023

I like the idea.

I'm thinking that by specifying the class we can look it up via CDI is just fallback to a no-args constructor if there is no bean.
I'll have a look some time this week.

@geoand
Copy link
Collaborator

geoand commented Nov 22, 2023

I was thinking of this, and I have a new proposal:

What if we keep what we have now, but change the default to something like: BeanIfExistsChatMemoryProviderSupplier which would automatically use the bean if it exists, but would not fail it no bean exists and would just not use any memory.

WDYT?

@cescoffier
Copy link
Collaborator Author

cescoffier commented Nov 22, 2023 via email

@geoand
Copy link
Collaborator

geoand commented Nov 22, 2023

I think Suppliers are used as factories

Yeah, that was my intention. If users want more advanced ways of choosing an implementation, then it's up to them to write the supplier.

@geoand
Copy link
Collaborator

geoand commented Nov 22, 2023

#51 should take care of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants