967 set useragent to lang chain4j

[roytruelove]

Issue

Set User Agent to 'LangChain4J'

Change

Added the User Agent wherever it was accessible

General checklist

• There are no breaking changes
• I have added unit and integration tests for my change
• I have manually run all the unit and integration tests in the module I have added/changed, and they are all green
• I have manually run all the unit and integration tests in the core and main modules, and they are all green

• [N/A] I have added/updated the documentation
• [N/A] I have added an example in the examples repo (only for "big" features)

Checklist for adding new model integration

• [N/A] I have added my new module in the BOM

Checklist for adding new embedding store integration

• [N/A] I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
• [N/A] I have added my new module in the BOM

Checklist for changing existing embedding store integration

• [N/A] I have manually verified that the {NameOfIntegration}EmbeddingStore works correctly with the data persisted using the latest released version of LangChain4j

[roytruelove]

A few questions for y'all

1. I've cut and pasted the same code to (so far) 3 of the OkHttpClient. Continuing this way doesn't feel right but there's not a natural place (that I've found) where I could create a singleton Interceptor and/or a make a constant for the User Agent value. Maybe a util class in dev.langchain4j.core but that doesn't have OkHttp as a dependency.

It seems like there are several other tickets discussing HTTP client including getting rid of OkHttp3 as the default, so perhaps the cut and paste is fine for the time being before larger changes take place?

2. There are no current tests for the classes I'm changing, so I'm wondering if as part of this change I'd need to create full coverage for all of the client classes?

[langchain4j]

Hi @roytruelove,

I've cut and pasted the same code to (so far) 3 of the OkHttpClient. Continuing this way doesn't feel right but there's not a natural place (that I've found) where I could create a singleton Interceptor and/or a make a constant for the User Agent value. Maybe a util class in dev.langchain4j.core but that doesn't have OkHttp as a dependency.
It seems like there are several other tickets discussing HTTP client including getting rid of OkHttp3 as the default, so perhaps the cut and paste is fine for the time being before larger changes take place?

Yes, I guess copy/paste is OK for now.

There are no current tests for the classes I'm changing, so I'm wondering if as part of this change I'd need to create full coverage for all of the client classes?

Ideally it would be nice to have a test. But if it takes too long, let's just test manually and see that existing tests are green.
If it is quick to do though, I would do it and copy/paste it everywhere.

[roytruelove]

Will do, thank you

…

On Thu, May 2, 2024 at 3:55 PM LangChain4j ***@***.***> wrote: Hi @roytruelove <https://github.com/roytruelove>, I've cut and pasted the same code to (so far) 3 of the OkHttpClient. Continuing this way doesn't feel right but there's not a natural place (that I've found) where I could create a singleton Interceptor and/or a make a constant for the User Agent value. Maybe a util class in dev.langchain4j.core but that doesn't have OkHttp as a dependency. It seems like there are several other tickets discussing HTTP client including getting rid of OkHttp3 as the default, so perhaps the cut and paste is fine for the time being before larger changes take place? Yes, I guess copy/paste is OK for now. There are no current tests for the classes I'm changing, so I'm wondering if as part of this change I'd need to create full coverage for all of the client classes? Ideally it would be nice to have a test. But if it takes too long, let's just test manually and see that existing tests are green. If it is quick to do though, I would do it and copy/paste it everywhere. — Reply to this email directly, view it on GitHub <#1046 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA6NMLJBVAGBY4H3BVMRULZAJAUZAVCNFSM6AAAAABHAYDWKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJQGU3DENBXGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[roytruelove]

Core code is complete, looking to see where I can add some tests

[roytruelove]

Added tests for new methods. Unfortunately it is a bigger lift to test that the HTTP calls contain the correct user agent, without adding mocks / inject-able HTTP clients. Please let me know your thoughts on this.

Other than that the PR is ready for review.

[roytruelove]

Modeled after InternalOpenAiHelper.

[roytruelove]

Interestingly this uses a beta verson of PredictionServiceSettings so it could not use the new InternalVertexAiHelper class.

[roytruelove]

Apologies - I marked this as "ready" when it should still be in draft..