Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.5, 6.0
    • Component/s: clients - java
    • Labels:
      None
    • Flags:
      Patch

      Description

      The REQUESTSTATUS API returns a "state" that is currently a String. This issue adds a RequestStatusState enum with the currently returned constants, for easier integration by clients. For backwards compatibility it parses the returned state in a lowercase form, as well resolve "notfound" to NOT_FOUND.

      1. SOLR-8560.patch
        52 kB
        Shai Erera
      2. SOLR-8560.patch
        49 kB
        Shai Erera

        Activity

        Hide
        Shai Erera added a comment -

        Patch introduces RequestStatusState and replaces code which did e.g. state.equals("completed") with state == RequestStatusState.COMPLETED. I also cleaned up CollectionsHandler.CollectionOperation.REQUESTSTATUS_OP a bit.

        Show
        Shai Erera added a comment - Patch introduces RequestStatusState and replaces code which did e.g. state.equals("completed") with state == RequestStatusState.COMPLETED . I also cleaned up CollectionsHandler.CollectionOperation.REQUESTSTATUS_OP a bit.
        Hide
        Anshum Gupta added a comment -

        Thanks Shai. Looks good overall other than the following:

        • Why reorder imports in AbstractFullDistribTestBase? The patched file has imports above the license header
        • CollectionsHandler.addStatusToResponse() does not use requestId. We should remove it from there
        • DistribDocExpirationUpdateProcessorTest has no real changes but just formatting. Also, it's completely unrelated to the issue.
        Show
        Anshum Gupta added a comment - Thanks Shai. Looks good overall other than the following: Why reorder imports in AbstractFullDistribTestBase? The patched file has imports above the license header CollectionsHandler.addStatusToResponse() does not use requestId. We should remove it from there DistribDocExpirationUpdateProcessorTest has no real changes but just formatting. Also, it's completely unrelated to the issue.
        Hide
        Shai Erera added a comment -

        Why reorder imports in AbstractFullDistribTestBase? The patched file has imports above the license header

        I changed the file and to remove some unused imports I "organized imports". Anyway, moved the header to above the package.

        CollectionsHandler.addStatusToResponse() does not use requestId. We should remove it from there

        Done.

        DistribDocExpirationUpdateProcessorTest has no real changes but just formatting. Also, it's completely unrelated to the issue.

        Guess I changed these while debugging something. Reverted.

        Show
        Shai Erera added a comment - Why reorder imports in AbstractFullDistribTestBase? The patched file has imports above the license header I changed the file and to remove some unused imports I "organized imports". Anyway, moved the header to above the package. CollectionsHandler.addStatusToResponse() does not use requestId. We should remove it from there Done. DistribDocExpirationUpdateProcessorTest has no real changes but just formatting. Also, it's completely unrelated to the issue. Guess I changed these while debugging something. Reverted.
        Hide
        Shai Erera added a comment -

        Patch addresses Anshum Gupta comments.

        Show
        Shai Erera added a comment - Patch addresses Anshum Gupta comments.
        Hide
        Anshum Gupta added a comment -

        +1

        Thanks Shai.

        I wish we could be consistent in our header placement throughout the project and have it as the first thing in the file. At least before the imports. Then we can rightly call it the 'header'

        Show
        Anshum Gupta added a comment - +1 Thanks Shai. I wish we could be consistent in our header placement throughout the project and have it as the first thing in the file. At least before the imports. Then we can rightly call it the 'header'
        Hide
        ASF subversion and git services added a comment -

        Commit 1726144 from Shai Erera in branch 'dev/trunk'
        [ https://svn.apache.org/r1726144 ]

        SOLR-8560: Add RequestStatusState enum

        Show
        ASF subversion and git services added a comment - Commit 1726144 from Shai Erera in branch 'dev/trunk' [ https://svn.apache.org/r1726144 ] SOLR-8560 : Add RequestStatusState enum
        Hide
        ASF subversion and git services added a comment -

        Commit 1726148 from Shai Erera in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1726148 ]

        SOLR-8560: Add RequestStatusState enum

        Show
        ASF subversion and git services added a comment - Commit 1726148 from Shai Erera in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1726148 ] SOLR-8560 : Add RequestStatusState enum
        Hide
        Shai Erera added a comment -

        Thanks Anshum Gupta for the review.

        Show
        Shai Erera added a comment - Thanks Anshum Gupta for the review.

          People

          • Assignee:
            Shai Erera
            Reporter:
            Shai Erera
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development