Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
        49 kB
        Shai Erera
      2. SOLR-8560.patch
        52 kB
        Shai Erera

        Activity

        Hide
        shaie Shai Erera added a comment -

        Thanks Anshum Gupta for the review.

        Show
        shaie Shai Erera added a comment - Thanks Anshum Gupta for the review.
        Hide
        jira-bot 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
        jira-bot 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
        jira-bot 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
        jira-bot 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
        anshumg 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
        anshumg 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
        shaie Shai Erera added a comment -

        Patch addresses Anshum Gupta comments.

        Show
        shaie Shai Erera added a comment - Patch addresses Anshum Gupta comments.
        Hide
        shaie 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
        shaie 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
        anshumg 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
        anshumg 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
        shaie 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
        shaie 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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development