Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9103

Restore ability for users to add custom Streaming Expressions

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.3
    • Component/s: None
    • Labels:
      None

      Description

      StreamHandler is an implicit handler. So to make it extensible, we can introduce the below syntax in solrconfig.xml.

      <expressible name="hello" class="org.apache.solr.search.HelloStream"/>
      

      This will add hello function to streamFactory of StreamHandler.

      1. HelloStream.class
        3 kB
        Cao Manh Dat
      2. SOLR-9103.patch
        16 kB
        Dennis Gove
      3. SOLR-9103.patch
        8 kB
        Dennis Gove
      4. SOLR-9103.PATCH
        11 kB
        Cao Manh Dat
      5. SOLR-9103.PATCH
        11 kB
        Cao Manh Dat

        Issue Links

          Activity

          Hide
          caomanhdat Cao Manh Dat added a comment -

          Initial patch along with test.

          Show
          caomanhdat Cao Manh Dat added a comment - Initial patch along with test.
          Hide
          caomanhdat Cao Manh Dat added a comment -

          Update patch file

          • Add dynamic loading testcase with custom stream.
          Show
          caomanhdat Cao Manh Dat added a comment - Update patch file Add dynamic loading testcase with custom stream.
          Hide
          dpgove Dennis Gove added a comment -

          This ability existed in the initial patch adding Streaming Expressions (SOLR-7377). I guess it was removed at some point, though I wonder why. Glad to see it added back!

          Show
          dpgove Dennis Gove added a comment - This ability existed in the initial patch adding Streaming Expressions ( SOLR-7377 ). I guess it was removed at some point, though I wonder why. Glad to see it added back!
          Hide
          joel.bernstein Joel Bernstein added a comment - - edited

          We need to the restore the ability for users to add their own expressions. But I'm not sure the solrconfig.xml is the best place to do this.

          How do people feel about adding a new expressions.properties file to the configset?

          Show
          joel.bernstein Joel Bernstein added a comment - - edited We need to the restore the ability for users to add their own expressions. But I'm not sure the solrconfig.xml is the best place to do this. How do people feel about adding a new expressions.properties file to the configset?
          Hide
          dsmiley David Smiley added a comment -

          Why not solrconfig.xml? It's a known config file used for a variety of miscellaneous configuration and hooks to be updated via HTTP APIs (with added work, granted). Unless there's a good reason to add another config file, it seems clearer to users to use an existing known spot.

          Show
          dsmiley David Smiley added a comment - Why not solrconfig.xml? It's a known config file used for a variety of miscellaneous configuration and hooks to be updated via HTTP APIs (with added work, granted). Unless there's a good reason to add another config file, it seems clearer to users to use an existing known spot.
          Hide
          joel.bernstein Joel Bernstein added a comment - - edited

          I'm thinking we should externalize all the function mappings rather then having them coded into classes. It just seems nicer to have a properties file to hold the entire Streaming Expressions function table rather then defining it in the solrconfig.xml. Also Streaming Expressions can be run outside of Solr, so having a properties file that can be pointed to would facilitate this.

          Show
          joel.bernstein Joel Bernstein added a comment - - edited I'm thinking we should externalize all the function mappings rather then having them coded into classes. It just seems nicer to have a properties file to hold the entire Streaming Expressions function table rather then defining it in the solrconfig.xml. Also Streaming Expressions can be run outside of Solr, so having a properties file that can be pointed to would facilitate this.
          Hide
          dsmiley David Smiley added a comment -

          We're just talking about mapping an invocation name to a class name, right? If so, then how is this different than a QParserPlugin or a DocumentTransformer, or ValueSource? Consistency in approach.

          Show
          dsmiley David Smiley added a comment - We're just talking about mapping an invocation name to a class name, right? If so, then how is this different than a QParserPlugin or a DocumentTransformer, or ValueSource? Consistency in approach.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          As long as the user can use the Config APIs to add/remove expressions without needing to edit any file then it does not matter where the mappings are actually persisted. Having said that keeping them in solrconfig.xml may be easier to integrate from a Config API perspective.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - As long as the user can use the Config APIs to add/remove expressions without needing to edit any file then it does not matter where the mappings are actually persisted. Having said that keeping them in solrconfig.xml may be easier to integrate from a Config API perspective.
          Hide
          joel.bernstein Joel Bernstein added a comment - - edited

          Ok, then let's use the solrconfig.xml for adding custom expressions. In that case I think this patch looks like a good approach. We can use the same approach as other pluggables and register the standard expressions in a class.

          Show
          joel.bernstein Joel Bernstein added a comment - - edited Ok, then let's use the solrconfig.xml for adding custom expressions. In that case I think this patch looks like a good approach. We can use the same approach as other pluggables and register the standard expressions in a class.
          Hide
          dpgove Dennis Gove added a comment -

          testDynamicLoadingCustomStream is not passing because it cannot find runtimecode/HelloStream.class. Note that I did add the file solr/core/src/test-files/runtimecode/HelloStream.java. But it doesn't appear the test can find the .class of that. I know you provided a .class but I'm not sure I'm comfortable adding a .class to the source code.

          The test does pass if I run it directly in Eclipse, however.

             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestCustomStream -Dtests.method=testDynamicLoadingCustomStream -Dtests.seed=96673E541CBCF992 -Dtests.slow=true -Dtests.locale=fr-CH -Dtests.timezone=Europe/Sarajevo -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
             [junit4] ERROR   28.5s | TestCustomStream.testDynamicLoadingCustomStream <<<
             [junit4]    > Throwable #1: java.lang.RuntimeException: Cannot find resource in classpath or in file-system (relative to CWD): runtimecode/HelloStream.class
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([96673E541CBCF992:E6D23776E646B225]:0)
             [junit4]    > 	at org.apache.solr.SolrTestCaseJ4.getFile(SolrTestCaseJ4.java:1798)
             [junit4]    > 	at org.apache.solr.core.TestDynamicLoading.getFileContent(TestDynamicLoading.java:261)
             [junit4]    > 	at org.apache.solr.core.TestCustomStream.testDynamicLoadingCustomStream(TestCustomStream.java:73)
             [junit4]    > 	at org.apache.solr.BaseDistributedSearchTestCase$ShardsRepeatRule$ShardsFixedStatement.callStatement(BaseDistributedSearchTestCase.java:985)
             [junit4]    > 	at org.apache.solr.BaseDistributedSearchTestCase$ShardsRepeatRule$ShardsStatement.evaluate(BaseDistributedSearchTestCase.java:960)
             [junit4]    > 	at java.lang.Thread.run(Thread.java:745)
          
          Show
          dpgove Dennis Gove added a comment - testDynamicLoadingCustomStream is not passing because it cannot find runtimecode/HelloStream.class. Note that I did add the file solr/core/src/test-files/runtimecode/HelloStream.java. But it doesn't appear the test can find the .class of that. I know you provided a .class but I'm not sure I'm comfortable adding a .class to the source code. The test does pass if I run it directly in Eclipse, however. [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestCustomStream -Dtests.method=testDynamicLoadingCustomStream -Dtests.seed=96673E541CBCF992 -Dtests.slow= true -Dtests.locale=fr-CH -Dtests.timezone=Europe/Sarajevo -Dtests.asserts= true -Dtests.file.encoding=ISO-8859-1 [junit4] ERROR 28.5s | TestCustomStream.testDynamicLoadingCustomStream <<< [junit4] > Throwable #1: java.lang.RuntimeException: Cannot find resource in classpath or in file-system (relative to CWD): runtimecode/HelloStream.class [junit4] > at __randomizedtesting.SeedInfo.seed([96673E541CBCF992:E6D23776E646B225]:0) [junit4] > at org.apache.solr.SolrTestCaseJ4.getFile(SolrTestCaseJ4.java:1798) [junit4] > at org.apache.solr.core.TestDynamicLoading.getFileContent(TestDynamicLoading.java:261) [junit4] > at org.apache.solr.core.TestCustomStream.testDynamicLoadingCustomStream(TestCustomStream.java:73) [junit4] > at org.apache.solr.BaseDistributedSearchTestCase$ShardsRepeatRule$ShardsFixedStatement.callStatement(BaseDistributedSearchTestCase.java:985) [junit4] > at org.apache.solr.BaseDistributedSearchTestCase$ShardsRepeatRule$ShardsStatement.evaluate(BaseDistributedSearchTestCase.java:960) [junit4] > at java.lang. Thread .run( Thread .java:745)
          Hide
          dpgove Dennis Gove added a comment -

          Was able to get the tests passing by moving the package of HelloStream to something that is compiled during test runs. This will prevent us from having to include a .class file in the source code.

          Show
          dpgove Dennis Gove added a comment - Was able to get the tests passing by moving the package of HelloStream to something that is compiled during test runs. This will prevent us from having to include a .class file in the source code.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 56cd8bffc6d9a166fe607bd228dca55370386122 in lucene-solr's branch refs/heads/master from Dennis Gove
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=56cd8bf ]

          SOLR-9103: Restore ability for users to add custom Streaming Expressions

          Show
          jira-bot ASF subversion and git services added a comment - Commit 56cd8bffc6d9a166fe607bd228dca55370386122 in lucene-solr's branch refs/heads/master from Dennis Gove [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=56cd8bf ] SOLR-9103 : Restore ability for users to add custom Streaming Expressions
          Hide
          dpgove Dennis Gove added a comment -

          Updated with some pre-commit changes.

          Show
          dpgove Dennis Gove added a comment - Updated with some pre-commit changes.
          Hide
          dpgove Dennis Gove added a comment -

          Cao, thank you very much for this. I'm really happy to see this feature restored.

          Show
          dpgove Dennis Gove added a comment - Cao, thank you very much for this. I'm really happy to see this feature restored.
          Hide
          joel.bernstein Joel Bernstein added a comment -

          I'm happy as well!

          Show
          joel.bernstein Joel Bernstein added a comment - I'm happy as well!
          Hide
          caomanhdat Cao Manh Dat added a comment - - edited

          Thanks Dennis for reviewing the patch.

          Show
          caomanhdat Cao Manh Dat added a comment - - edited Thanks Dennis for reviewing the patch.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5e6a8e7c7537bfe1d2fdabf3b1bdd9fe825c5996 in lucene-solr's branch refs/heads/branch_6x from Dennis Gove
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5e6a8e7 ]

          SOLR-9103: Restore ability for users to add custom Streaming Expressions

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5e6a8e7c7537bfe1d2fdabf3b1bdd9fe825c5996 in lucene-solr's branch refs/heads/branch_6x from Dennis Gove [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5e6a8e7 ] SOLR-9103 : Restore ability for users to add custom Streaming Expressions

            People

            • Assignee:
              dpgove Dennis Gove
              Reporter:
              caomanhdat Cao Manh Dat
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development