[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1684: SOLR-14613: strongly typed placement plugin interface and implementation

[GitBox <gi...@apache.org>]

noblepaul edited a comment on pull request #1684:
URL: https://github.com/apache/lucene-solr/pull/1684#issuecomment-686359748


   > By all means, if you disagree so strongly with the approach presented here then please do so
   
   I clearly have objections to the verbosity of the proposals here. This does not mean that what I propose will be perfect. Nobody can design "the perfect system". A perfect system does not exist. I'm trying to give suggestions so that we can eliminate those obvious problems.
   
   At this point, two of you are invested in this feature and I totally understand your compulsions. 
   
   > The impl. can be barebones, but the API that we define is going to stay with us for the next couple years at least, so it pays off to think seriously how to do it in an elegant and efficient way.
   
   What I see is a largescale undertaking to rewrite everything that we have in Solr . How else will you justify the presence of a class such as this https://github.com/apache/lucene-solr/blob/e385e7083bd4e0263e563d35555cf0cb53b0e03e/solr/core/src/java/org/apache/solr/cluster/placement/Replica.java
   in  a framework that is designed to just find out nodes to place replicas in a cluster?
   
   We do not want to support such a massive framework . 
   I strongly recommend that we do not make duplicates for what we have today just for this particular framework. We already have an `AssignStrategy` interface. AFAIK, the only place where we have problem is in the `NodeStateProvider` . Let's add a method `NodeStateProvider#getPropertyFetcher()` and a new interface `NodePropertyFetcher`  and start reusing the `AssignStrategy` interface itself. That way we do not need any new framework.  Most likely there won't be too many implementations of this framework. Now, if you wish me to make a PR with this suggestion, I'm happy to do that
   
   Just in case you wish to load this plugin from packages , My PR (#1684 ) has the necessary code to load this prom package. (That's optional at this point)
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org