solr icon indicating copy to clipboard operation
solr copied to clipboard

Refactor SolrZkclient to use a Builder pattern

Open noblepaul opened this issue 2 years ago • 1 comments

noblepaul avatar Sep 15 '22 04:09 noblepaul

Just a quick comment: Does the move to Overseer render SolrZkClient obsolete @HoustonPutman ? Thus should we bother with changing SolrZkClient if it's going away any month now? Maybe this is a 10x vs 9x thing.

dsmiley avatar Sep 23 '22 21:09 dsmiley

Just a quick comment: Does the move to Overseer render SolrZkClient obsolete @HoustonPutman ? Thus should we bother with changing SolrZkClient if it's going away any month now? Maybe this is a 10x vs 9x thing.

Do you mean the move to Curator? If so, then no. At least in my branch, that would have been far too big of a refactor. Maybe sometime in the future, but this is worthwhile and will actually make the Curator changes to SolrZkClient cleaner.

HoustonPutman avatar Oct 20 '22 15:10 HoustonPutman

Could we follow the same pattern defined in SOLR-16595 and SOLR-16590 of using with instead of bare property names, and using TimeUnit? Have some consistency in how we create our builders! I'm happy to do the leg work if you don't want to do it, but are good with the idea ;-) Just let me know.

epugh avatar Feb 09 '23 15:02 epugh

@noblepaul The change in TestCloudManagedSchema is clearly wrong. Notice the "(" in the string. This test is failing on me and Jenkins now.

dsmiley avatar Feb 14 '23 05:02 dsmiley

fixing...

noblepaul avatar Feb 14 '23 07:02 noblepaul

Still getting (a smaller amount of) test failures: https://ci-builds.apache.org/job/Solr/job/Solr-Check-main/6093/#showFailuresLink

HoustonPutman avatar Feb 14 '23 16:02 HoustonPutman