Solr
  1. Solr
  2. SOLR-2105

Rename RequestHandler param 'update.processor' to 'update.chain'.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4.1
    • Fix Version/s: 3.2, 4.0-ALPHA
    • Component/s: update
    • Labels:
      None

      Description

      Today we reference a custom updateRequestProcessorChain using the update request parameter "update.processor".
      See http://wiki.apache.org/solr/SolrConfigXml#UpdateRequestProcessorChain_section

      This is confusing, since what we are really referencing is not an UpdateProcessor, but an updateRequestProcessorChain.

      I propose that "update.processor" is renamed as "update.chain" or similar

      1. SOLR-2105.patch
        13 kB
        Mark Miller
      2. SOLR-2105.patch
        10 kB
        Jan Høydahl
      3. SOLR-2105.patch
        6 kB
        Jan Høydahl
      4. SOLR-2105-remove-deprecation-in-trunk.patch
        10 kB
        Jan Høydahl
      5. SOLR-2105-remove-deprecation-in-trunk.patch
        10 kB
        Jan Høydahl
      6. SOLR-2105-remove-deprecation-in-trunk.patch
        10 kB
        Jan Høydahl

        Issue Links

          Activity

          Hide
          Jan Høydahl added a comment -

          The attached patch renames the parameter, both in code and config. Tests run after applying it, but I have not done regression testing of the functionality.

          Show
          Jan Høydahl added a comment - The attached patch renames the parameter, both in code and config. Tests run after applying it, but I have not done regression testing of the functionality.
          Hide
          Jan Høydahl added a comment -

          5-minute fix candidate for 3.1

          Anyone vote for including this name change fix in the 3.1 release?
          Custom update chains are very little in use out there so it's easier to change the name of the parameter now than later. Marking this change clearly in CHANGES.TXT should let anyone be able to catch up. A softer option is to leave the old param in there but deprecated.

          Show
          Jan Høydahl added a comment - 5-minute fix candidate for 3.1 Anyone vote for including this name change fix in the 3.1 release? Custom update chains are very little in use out there so it's easier to change the name of the parameter now than later. Marking this change clearly in CHANGES.TXT should let anyone be able to catch up. A softer option is to leave the old param in there but deprecated.
          Hide
          Mark Miller added a comment -

          I like this change.

          Can you leave update.processor in but deprecated? Perhaps print log warning if it's detected.

          Then we could make a hard change in 4.X perhaps?

          Show
          Mark Miller added a comment - I like this change. Can you leave update.processor in but deprecated? Perhaps print log warning if it's detected. Then we could make a hard change in 4.X perhaps?
          Hide
          Ryan McKinley added a comment -

          +1

          Show
          Ryan McKinley added a comment - +1
          Hide
          Jan Høydahl added a comment - - edited

          Updated patch attached.

          • Use of update.processor is now deprecated, logging a warning (instead of removing as in previous patch)
          • Added test case which tests that both params work

          Patch is for trunk.

          Show
          Jan Høydahl added a comment - - edited Updated patch attached. Use of update.processor is now deprecated, logging a warning (instead of removing as in previous patch) Added test case which tests that both params work Patch is for trunk.
          Hide
          Jan Høydahl added a comment -

          Mark, did you have a chance to test the latest patch?

          Show
          Jan Høydahl added a comment - Mark, did you have a chance to test the latest patch?
          Hide
          Mark Miller added a comment -

          Thanks a lot Jan - nice work. Sorry it took me a while to get back to this - did not want to try and cram it into 3.1.

          Here is a patch for trunk. I'll backport to 3.X soon and we can resolve this.

          Show
          Mark Miller added a comment - Thanks a lot Jan - nice work. Sorry it took me a while to get back to this - did not want to try and cram it into 3.1. Here is a patch for trunk. I'll backport to 3.X soon and we can resolve this.
          Hide
          Mark Miller added a comment -

          Thanks Jan!

          Show
          Mark Miller added a comment - Thanks Jan!
          Hide
          Markus Jelsma added a comment -

          Excellent job for printing the deprecation warning, i seem to have overlooked this issue!

          Show
          Markus Jelsma added a comment - Excellent job for printing the deprecation warning, i seem to have overlooked this issue!
          Hide
          Robert Muir added a comment -

          Bulk close for 3.2

          Show
          Robert Muir added a comment - Bulk close for 3.2
          Hide
          Jan Høydahl added a comment -

          Re-opening for 4.x, as I believe we can remove the deprecation warning and remove support for 'update.processor' completely.

          Show
          Jan Høydahl added a comment - Re-opening for 4.x, as I believe we can remove the deprecation warning and remove support for 'update.processor' completely.
          Hide
          Jan Høydahl added a comment -

          Attaching patch which removes deprecation from 4.x, so that only "update.chain" is now supported. Will commit in a day or two if no comments.

          Show
          Jan Høydahl added a comment - Attaching patch which removes deprecation from 4.x, so that only "update.chain" is now supported. Will commit in a day or two if no comments.
          Hide
          Jan Høydahl added a comment -

          Same patch with ASF license grant

          Show
          Jan Høydahl added a comment - Same patch with ASF license grant
          Hide
          Mark Miller added a comment -

          Will commit in a day or two

          +1

          Do we have a big fat warning for this in the upgrade section of Changes yet?

          Show
          Mark Miller added a comment - Will commit in a day or two +1 Do we have a big fat warning for this in the upgrade section of Changes yet?
          Hide
          Jan Høydahl added a comment -

          Added big fat warning in upgrade section

          Show
          Jan Høydahl added a comment - Added big fat warning in upgrade section
          Hide
          Jan Høydahl added a comment -

          Checked in the fix for trunk, now "update.processor" param is long gone

          Show
          Jan Høydahl added a comment - Checked in the fix for trunk, now "update.processor" param is long gone

            People

            • Assignee:
              Mark Miller
              Reporter:
              Jan Høydahl
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development