SOLR-17611 SolrJ User-Agent, pass Solr version #2927
Conversation
|
There are asserts in the unit tests for the user-agent, so changing it will cause failures I think. See |
| public static final String REQ_PRINCIPAL_KEY = "solr-req-principal"; | ||
| private static final String USER_AGENT = | ||
| "Solr[" |
There was a problem hiding this comment.
Could this go in the superclass so it doesn't have to exist twice?
There was a problem hiding this comment.
I'd hate to create constants merely for the parts of this string that we concatenate -- I hate that coding practice, to be blunt. I could see a utility method that composes them. But any way it's tested so I'm not sure what the risk/concern of not doing what you suggest is. This is a one-liner BTW.
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
Show resolved
Hide resolved
| return null; | ||
| } | ||
| try { | ||
| return SolrVersion.valueOf(header.substring(header.lastIndexOf(' ') + 1)); |
There was a problem hiding this comment.
If this gets an older client it will return 1.0 or 2.0, which can be somewhat misleading.
There was a problem hiding this comment.
Yeah; I thought it would have no consequence so I did nothing about it. At least by returning a SolrVersion instead of null, we communicate indirectly that it's at least SolrJ. I could add a javadoc comment on 1.0 and 2.0 being used prior to 9.8.
There was a problem hiding this comment.
I added this to the docs.
There was a problem hiding this comment.
BTW that method has no callers; isn't tested. I did manually test it. Obviously I intend to have it be used soon.
|
Ready to merge; will do within a couple days if I hear nothing. |
SolrJ's HTTP User-Agent header now uses the version of SolrJ (Solr) instead of 1.0 or 2.0. There's a corresponding HttpSolrCall.getUserAgentSolrVersion to parse it. (cherry picked from commit 32aee2d)
https://issues.apache.org/jira/browse/SOLR-17611 (read this for context)
No tests. I suppose the best test would be something in the smoke test which is at release time where we definitely have the "specification version" in the JAR manifest. Once we have something that detects the version to behave differently, we will need it to work in a unit test environment too, where we want the latest behavior but don't have a spec version in a manifest.
Maybe the PR should include something in SolrVersion to parse it as well?