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 2019/12/20 22:15:16 UTC
[GitHub] [lucene-solr] noblepaul opened a new pull request #1108: SOLR-14125
: Streaming expressions to be loadable from packages
noblepaul opened a new pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108
DO NOT MERGE . Work in progress
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] joel-bernstein commented on a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360742212
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
Ok, this makes sense.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul removed a comment on issue #1108:
SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul removed a comment on issue #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#issuecomment-568348892
SOLR-14125: Make <expressible> plugins work with packages
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360733385
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
In the new scheme of things you can't hold direct reference to a `class`.
If a plugin can be reloaded, it's best to just hold an indirect reference. The `Supplier` makes it cleaner. For the in-built classes, the supplier is just a simple wrapper.
But, for the plugins loaded from packages, it listens to package changes and gives the right class on-demand. The `Class` objects of a plugin are something that needs to be garbage collected as the `ClassLoader` goes away. If there is a Strong reference to a `Class` object, the `ClassLoader` will not be garbage collected
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360733385
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
In the new scheme of things you can't hold direct reference to a `class`.
If a plugin can be reloaded, it's best to just hold an indirect reference. The `Supplier` makes it cleaner. For the in-built classes, the supplier is just a simple wrapper. But for the plugins loaded from packages, it is listens to package changes and gives the right class on-demand
However, it's possible to not have a `Supplier` and directly replace the `Class` Object. It's just not clean.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360733385
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
In the new scheme of things you can't hold direct reference to a `class`.
If a plugin can be reloaded, it's best to just hold an indirect reference. The `Supplier` makes it cleaner. For the in-built classes, the supplier is just a simple wrapper. But for the plugins loaded from packages, it is listens to package changes and gives the right class on-demand
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1108: SOLR-14125 :
Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on issue #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#issuecomment-568348892
SOLR-14125: Make <expressible> plugins work with packages
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #1108: SOLR-14125 :
Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on issue #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#issuecomment-568223643
It's in a good shape. reviews are welcome
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] joel-bernstein commented on a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360722642
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
Why the use of the Supplier? Is this a cosmetic change or is there some functionality here that I'm not seeing? I'm not against a cosmetic change but changes to StreamFactory are going to slow down landing this patch quite a bit because it's such an important class for Streaming Expressions. So if it's purely cosmetic and we want to land the other changes faster, let's do this in another ticket.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #1108: SOLR-14125 :
Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul merged pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] joel-bernstein commented on a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
joel-bernstein commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360722642
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
Why the use of the Supplier? Is this a cosmetic change or is there some functionality here that I'm not seeing? I'm not against a cosmetic change but changes to StreamFactory are going to slow down landing this patch quite a bit because it's such an important class for the Streaming Expressions. So if it's purely cosmetic and we want to land the other changes faster, let's do this in another ticket.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360733385
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
In the new scheme of things you can't hold direct reference to a `class`.
If a plugin can be reloaded, it's best to just hold an indirect reference. The `Supplier` makes it cleaner. For the in-built classes, the supplier is just a simple wrapper. But for the plugins loaded from packages, it listens to package changes and gives the right class on-demand. The `Class` objects of a plugin are something that needs to be garbage collected as the `ClassLoader` goes away.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 a change in pull request
#1108: SOLR-14125 : Streaming expressions to be loadable from packages
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1108: SOLR-14125 : Streaming expressions to be loadable from packages
URL: https://github.com/apache/lucene-solr/pull/1108#discussion_r360733385
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java
##########
@@ -24,8 +24,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import java.util.Map.Entry;
+import java.util.function.Supplier;
Review comment:
In the new scheme of things you can't hold direct reference to a `class`.
If a plugin can be reloaded, it's best to just hold an indirect reference. The `Supplier` makes it cleaner. For the in-built classes, the supplier is just a simple wrapper. But for the plugins loaded from packages, it listens to package changes and gives the right class on-demand
However, it's possible to not have a `Supplier` and directly replace the `Class` Object. It's just not clean.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org