You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/10 02:20:46 UTC

[GitHub] [lucene-solr] noblepaul opened a new pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

noblepaul opened a new pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730


   new PR for #1694 


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


[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671651150


   >what is the target use case of the interface and lazy implementation? 
   
   The objectives are many
   
   -  Totally refactor Solr code base to minimize dependencies on concrete classes. This enables us to do simulation and testing, make code more readable, and enable refactoring
   - As we move to  a new mode for Solr with a lean core and packages/plugins, we want to have less API surface area against which the plugins are written. This enables the plugins to work against a wider range of versions without rewriting/recompiling
   - The `LazySolrCluster` will be the default impl for these interfaces. Because, this is the current behaviour (mostly). We expect fresh data to be available all times
   
   The problem with the existing classes implementing the interfaces is that, users of the APIs will cast these objects to the underlying concrete classes, which defeats the purpose
    


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


[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671229892


   > Separating out the new lazy implementations into another PR and keeping this one for adding interfaces to internal classes would have made reviewing easier.
   
   Yeah, that was the other PR #1694 , I have revived it
   
   @murblanc please review it 
   
   >Are there places in the code where currently the concrete classes are used and that could be changed to use the interfaces instead? In other words, how/where would these interfaces be used?
   
   The current concrete classes do not use/implement these interfaces. These interfaces will only be a part of implementations. for instance, the `LazySolrCluster` is one of the impl. In the future we should add a couple 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.

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


[GitHub] [lucene-solr] noblepaul commented on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671263417


   I realized that I had to tweak the APIs after writing an implementation. I have updated the API-only PR #1694 to reflect the latest
    


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


[GitHub] [lucene-solr] chatman commented on pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671214349


   I'm actively reviewing this. Please give me 2 days more before merging.


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


[GitHub] [lucene-solr] murblanc commented on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671509879


   
   > The current concrete classes do not use/implement these interfaces. These interfaces will only be a part of implementations. for instance, the `LazySolrCluster` is one of the impl. In the future we should add a couple more
   
   @noble what is the target use case of the interface and lazy implementation? I thought your aim was to create interfaces to existing internal classes therefore I expected them to implement these interfaces so these interfaces to be used in the code replacing the actual classes...
   Maybe it's just me not understading your intention 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.

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


[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671651150


   >what is the target use case of the interface and lazy implementation? 
   
   The objectives are many
   
   - The `LazySolrCluster` will be the default impl for these interfaces. Because, this is the current behaviour (mostly). We expect fresh data to be available all times
   -  Totally refactor Solr code base to minimize dependencies on concrete classes. This enables us to do simulation and testing, make code more readable, and enable refactoring
   - As we move to  a new mode for Solr with a lean core and packages/plugins, we want to have less API surface area against which the plugins are written. This enables the plugins to work against a wider range of versions without rewriting/recompiling
   
   The problem with the existing classes implementing the interfaces is that, users of the APIs will cast these objects to the underlying concrete classes, which defeats the purpose
    


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


[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671229892


   > Separating out the new lazy implementations into another PR and keeping this one for adding interfaces to internal classes would have made reviewing easier.
   
   Yeah, that was the other PR #1694 , I have revived it
   
   >Are there places in the code where currently the concrete classes are used and that could be changed to use the interfaces instead? In other words, how/where would these interfaces be used?
   
   The current concrete classes do not use/implement these interfaces. These interfaces will only be a part of implementations. for instance, the `LazySolrCluster` is one of the impl. In the future we should add a couple 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.

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


[GitHub] [lucene-solr] murblanc commented on pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671219903


   I could only look briefly on the phone screen (vacation) but two comments:
   
   Separating out the new lazy implementations into another PR and keeping this one for adding interfaces to internal classes would have made reviewing easier.
   
   Are there places in the code where currently the concrete classes are used and that could be changed to use the interfaces instead? In other words, how/where would these interfaces be used? 


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


[GitHub] [lucene-solr] noblepaul merged pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul merged pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730


   


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


[GitHub] [lucene-solr] noblepaul commented on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671651150


   >what is the target use case of the interface and lazy implementation? 
   
   The objectives are many
   
   -  Totally refactor Solr code base to minimize dependencies on concrete classes. This enables us to do simulation and testing, make code more readable, and enable refactoring
   - As we move to  a new mode for Solr with a lean core and packages/plugins, we want to have less API surface area against which the plugins are written. This enables us the plugins to work against a wider range of versions without rewriting/recompiling
   
   The problem with the existing classes implementing the interfaces is that, users of the APIs will cast these objects to the underlying concrete classes, which defeats the purpose
    


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


[GitHub] [lucene-solr] noblepaul edited a comment on pull request #1730: SOLR-14680: Provide an implementation for the new SolrCluster API

Posted by GitBox <gi...@apache.org>.
noblepaul edited a comment on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671651150


   >what is the target use case of the interface and lazy implementation? 
   
   The objectives are many
   
   -  Totally refactor Solr code base to minimize dependencies on concrete classes. This enables us to do simulation and testing, make code more readable, and enable refactoring
   - As we move to  a new mode for Solr with a lean core and packages/plugins, we want to have less API surface area against which the plugins are written. This enables the plugins to work against a wider range of versions without rewriting/recompiling
   
   The problem with the existing classes implementing the interfaces is that, users of the APIs will cast these objects to the underlying concrete classes, which defeats the purpose
    


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


[GitHub] [lucene-solr] noblepaul commented on pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671136587


   Planning to commit this soon


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


[GitHub] [lucene-solr] noblepaul commented on pull request #1730: SOLR-14680: Provide simple interfaces to our concrete SolrCloud classes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1730:
URL: https://github.com/apache/lucene-solr/pull/1730#issuecomment-671229892


   > Separating out the new lazy implementations into another PR and keeping this one for adding interfaces to internal classes would have made reviewing easier.
   
   Yeah, that was the other PR #1694 , I shall revive it. 
   
   >Are there places in the code where currently the concrete classes are used and that could be changed to use the interfaces instead? In other words, how/where would these interfaces be used?
   
   The current concrete classes do not use/implement these interfaces. These interfaces will only be a part of implementations. for instance, the `LazySolrCluster` is one of the impl. In the future we should add a couple 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.

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