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

DIH doesn't use its own interface + writerImpl has no information about the request

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA, 4.0-BETA
    • Fix Version/s: 4.9
    • Labels:
      None

      Description

      The use case: I would like to extend DIH by providing a new writer, I have tried everything but can't accomplish it without either a) duplicating whole DIHandler or b) java reflection tricks. Almost everything inside DIH is private and the mechanism to instantiate a new writer based on the 'writerImpl' mechanism seems lacking important functionality

      It doesn't give the new class a chance to get information about the request, update processor. Also, the writer is instantiated twice (when 'writerImpl' is there), which is really unnecessary.

      As a solution, the existing DIHandler.getSolrWriter() should instantiate the appropriate writer and send it to DocBuilder (it is already doing that for SolrWriter). And DocBuilder doesn't need to create a second (duplicate) writer

      1. SOLR-3671.patch
        11 kB
        James Dyer
      2. SOLR-3671.patch
        12 kB
        Roman Chyla

        Issue Links

          Activity

          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1597937 from Michael McCandless in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1597937 ]

          SOLR-3671: fix ongoing smoke test build failure

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1597937 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1597937 ] SOLR-3671 : fix ongoing smoke test build failure
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1597936 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1597936 ]

          SOLR-3671: fix ongoing smoke test build failure

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1597936 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1597936 ] SOLR-3671 : fix ongoing smoke test build failure
          Hide
          jdyer James Dyer added a comment -

          Thanks, Roman!

          Show
          jdyer James Dyer added a comment - Thanks, Roman!
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1592585 from jdyer@apache.org in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1592585 ]

          SOLR-3671: DIHWriter fix

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1592585 from jdyer@apache.org in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1592585 ] SOLR-3671 : DIHWriter fix
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1592583 from jdyer@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1592583 ]

          SOLR-3671: DIHWriter fix

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1592583 from jdyer@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1592583 ] SOLR-3671 : DIHWriter fix
          Hide
          aaronlab Aaron LaBella added a comment -

          James,

          I downloaded the patch and refactored my code/test case and everything works fine. Can you go ahead and commit that to branch_4x? Your patch cleans up the code to use the DIHWriter interface, which is better anyhow.

          Thanks.

          Show
          aaronlab Aaron LaBella added a comment - James, I downloaded the patch and refactored my code/test case and everything works fine. Can you go ahead and commit that to branch_4x? Your patch cleans up the code to use the DIHWriter interface, which is better anyhow. Thanks.
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          +1 LGTM

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - +1 LGTM
          Hide
          jdyer James Dyer added a comment -

          Updated patch. All tests pass.

          Show
          jdyer James Dyer added a comment - Updated patch. All tests pass.
          Hide
          jdyer James Dyer added a comment -

          This is a nice-to-have but doesn't need to be rushed into 4.0. Removing the fix version and changing to "minor".

          Show
          jdyer James Dyer added a comment - This is a nice-to-have but doesn't need to be rushed into 4.0. Removing the fix version and changing to "minor".
          Hide
          hossman Hoss Man added a comment -

          assigning to james to assess if this is a blocker for 4.0 or should be punted

          Show
          hossman Hoss Man added a comment - assigning to james to assess if this is a blocker for 4.0 or should be punted
          Hide
          hossman Hoss Man added a comment -

          removing fixVersion=4.0 since there is no evidence that anyone is currently working on this issue. (this can certainly be revisited if volunteers step forward)

          also assigning to James Dyer for patch review.

          Show
          hossman Hoss Man added a comment - removing fixVersion=4.0 since there is no evidence that anyone is currently working on this issue. (this can certainly be revisited if volunteers step forward) also assigning to James Dyer for patch review.
          Hide
          rcmuir Robert Muir added a comment -

          rmuir20120906-bulk-40-change

          Show
          rcmuir Robert Muir added a comment - rmuir20120906-bulk-40-change

            People

            • Assignee:
              jdyer James Dyer
              Reporter:
              rchyla Roman Chyla
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development