You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/09/09 20:35:29 UTC
[GitHub] [solr] HoustonPutman opened a new pull request, #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
HoustonPutman opened a new pull request, #1004:
URL: https://github.com/apache/solr/pull/1004
https://issues.apache.org/jira/browse/SOLR-16404
Still not completely sure about the new convenience client builder in solrj-zookeeper. Will think about it more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a diff in pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #1004:
URL: https://github.com/apache/solr/pull/1004#discussion_r978011019
##########
solr/prometheus-exporter/build.gradle:
##########
@@ -22,6 +22,7 @@ description = 'Prometheus exporter for exposing metrics from Solr using Metrics
dependencies {
implementation project(':solr:solrj')
+ runtimeOnly project(':solr:solrj-zookeeper')
// ideally remove ZK dep
implementation('org.apache.zookeeper:zookeeper', {
Review Comment:
I would need to change solrj-zookeeper for that. I'll do it in a separate PR/Issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1244201570
Your changes explicitly refer to `BaseHttpSolrClient` but that's kind of sad to me; I view this class as an implementation detail to a subclass. Can you undo that? As I look at BaseHttpSolrClient; I think it should cease to exist. Its contents are static inner classes that appear suitable in SolrClient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a diff in pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #1004:
URL: https://github.com/apache/solr/pull/1004#discussion_r978011734
##########
solr/prometheus-exporter/build.gradle:
##########
@@ -22,6 +22,7 @@ description = 'Prometheus exporter for exposing metrics from Solr using Metrics
dependencies {
implementation project(':solr:solrj')
+ runtimeOnly project(':solr:solrj-zookeeper')
// ideally remove ZK dep
implementation('org.apache.zookeeper:zookeeper', {
Review Comment:
ahhh fixed the changelog entry
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1247124002
> > Looking at @dsmiley comment, does that suggest that the "how we use newer clients" is still being worked out a bit?
>
> I don't think "how we use newer clients" is still being worked out.
Yeah, I just need to change everything to use the new client, instead of the abstraction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1004:
URL: https://github.com/apache/solr/pull/1004#discussion_r978003746
##########
solr/prometheus-exporter/build.gradle:
##########
@@ -22,6 +22,7 @@ description = 'Prometheus exporter for exposing metrics from Solr using Metrics
dependencies {
implementation project(':solr:solrj')
+ runtimeOnly project(':solr:solrj-zookeeper')
// ideally remove ZK dep
implementation('org.apache.zookeeper:zookeeper', {
Review Comment:
@HoustonPutman The changes say removed the Zookeeper dependency, but looks like it is still here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1004:
URL: https://github.com/apache/solr/pull/1004#discussion_r978131765
##########
solr/CHANGES.txt:
##########
@@ -89,6 +89,8 @@ Improvements
* SOLR-16361: mod() is now accurate for all integers, floats, doubles and longs upto 2^52 (Dan Rosher via Eric Pugh)
+* SOLR-16404: Prometheus Exporter: Use non-deprecated Solr clients. (Houston Putman)
Review Comment:
From the user's point of view, you've switched to HTTP2; right? That's more interesting to readers than what's deprecated and not; otherwise it's a refactoring (no real impact).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1254217080
Fair enough, I've split off this which is necessary for the tests to pass once we make the prom-exp changes. https://github.com/apache/solr/pull/1036
I'll revert the other code and do it another time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1255434177
@dsmiley Only prometheus exporter code is touched now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman merged pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman merged PR #1004:
URL: https://github.com/apache/solr/pull/1004
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] HoustonPutman commented on a diff in pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #1004:
URL: https://github.com/apache/solr/pull/1004#discussion_r978853742
##########
solr/CHANGES.txt:
##########
@@ -89,6 +89,8 @@ Improvements
* SOLR-16361: mod() is now accurate for all integers, floats, doubles and longs upto 2^52 (Dan Rosher via Eric Pugh)
+* SOLR-16404: Prometheus Exporter: Use non-deprecated Solr clients. (Houston Putman)
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1246664043
I tried to dig into SOLR-16368, and it was hard... I was hoping this PR would be a good model for what the upgrade path should look like! Looking at @dsmiley comment, does that suggest that the "how we use newer clients" is still being worked out a bit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #1004: SOLR-16404: Use newer clients in the Prometheus Exporter
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1004:
URL: https://github.com/apache/solr/pull/1004#issuecomment-1246707836
I don't think "how we use newer clients" is still being worked out.
To be clear SOLR-16368 is a pure refactoring; no actual change in which SolrClients are actually being used. It's also a "best-effort" change -- do the easy cases. SOLR-16367 will change to use different clients; perhaps you refered to that one? Just do some easy cases first and some harder ones can be in other PRs. We might choose to change some API details to make switching easier but that'd probably deserve its own issue(s).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org