-
Notifications
You must be signed in to change notification settings - Fork 6
better User-Agent string
#873
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
base: rc-v0.5.15
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| @NoArgsConstructor(onConstructor_ = @Inject) | ||
| @Log | ||
| public class HealthCheckRequestHandler { | ||
|
|
Copilot
AI
Apr 8, 2025
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.
Since JAVA_SOURCE_CODE_VERSION is now public and used to build the User-Agent string, consider updating its Javadoc to indicate its intended external use.
| /** | |
| * The version of the Java source code. | |
| * This constant is used to build the User-Agent string and is intended for external use. | |
| */ |
| } | ||
|
|
||
| private String getProxyUserAgentString() { | ||
| //q: something better here? pass through the actual client's User-Agent value?? |
Copilot
AI
Apr 8, 2025
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.
[nitpick] Remove or clarify the commented question to avoid confusion; if it's a placeholder for future work, consider adding a TODO with clear context.
| //q: something better here? pass through the actual client's User-Agent value?? | |
| //TODO: Consider passing through the actual client's User-Agent value for better accuracy. |
|
|
||
| private String getProxyUserAgentString() { | ||
| //q: something better here? pass through the actual client's User-Agent value?? | ||
| return "Psoxy/" + HealthCheckRequestHandler.JAVA_SOURCE_CODE_VERSION + " (API Data Sanitization; Java/" + System.getProperty("java.version", "unknown") + ")"; |
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.
probably this can be static; so no need to calculate this again and again for each request
| ComposedHttpRequestInitializer initializer = | ||
| ComposedHttpRequestInitializer.of(initializeWithCredentials, | ||
| new GzipedContentHttpRequestInitializer("Psoxy")); | ||
| new GzipedContentHttpRequestInitializer(getProxyUserAgentString())); |
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.
I don't think this does anything at all, as is used as fallback
so presumably we are sending this instead always:
Also, should that class be Gzipped... with double P?
Features
User-Agentstring sent to API serversChange implications