Details

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

      Description

      I'd reviewed REST API code, and realized that it is old and clunky. I want make some refactoring of this part and propose these changes as patch.

      1. api_refactoring.patch
        65 kB
        Ivan Vershinin
      2. api_refactoring3.patch
        113 kB
        Fjodor Vershinin

        Issue Links

          Activity

          Hide
          talat Talat UYARER added a comment -

          Hi Ivan Vershinin,

          I am glad of your improvement. If you need help or anything else, please don't hesitance for telling us.

          Show
          talat Talat UYARER added a comment - Hi Ivan Vershinin , I am glad of your improvement. If you need help or anything else, please don't hesitance for telling us.
          Hide
          lewismc Lewis John McGibbney added a comment -

          Ivan Vershinin, as you maybe know Fjodor Vershinin is working on GSoC this year. Maybe this can be included in his work?
          What are you observations regarding current API weaknesses and what improvements can you suggest?
          Thanks

          Show
          lewismc Lewis John McGibbney added a comment - Ivan Vershinin , as you maybe know Fjodor Vershinin is working on GSoC this year. Maybe this can be included in his work? What are you observations regarding current API weaknesses and what improvements can you suggest? Thanks
          Hide
          lewismc Lewis John McGibbney added a comment -

          Assigned to Fjodor Vershinin for karma. It is most likely that he may geton to this whilst we work on GSoC.
          Fjodor Vershinin if you don't want this assigned then I can remove.

          Show
          lewismc Lewis John McGibbney added a comment - Assigned to Fjodor Vershinin for karma. It is most likely that he may geton to this whilst we work on GSoC. Fjodor Vershinin if you don't want this assigned then I can remove.
          Hide
          vershinin Ivan Vershinin added a comment -

          Hi guys!
          I've finished code cleanup and refactoring in this patch. Some things I want to mention:
          1) Static references (tightly coupled code). I had replaced it with DI.
          2) Strange way to work with exceptions. I've removed exception declarations from method signatures, and replaced some checked exceptions with runtime ones.
          3) Guava methods to create collections/maps, such as Maps.newHashMap() makes code cleaner.
          4) Some big methods had been split into small parts.
          5) Executor and job factory classes has been extracted.


          For Fjodor Vershinin, you can try to replace non type-safe code for getting query parameters with JAX-RS. (Java API for restful services). This can give two benefits: type-safety and code reuse (you can reuse jax-rs models in your client, where proxy can be generated automatically).

          Show
          vershinin Ivan Vershinin added a comment - Hi guys! I've finished code cleanup and refactoring in this patch. Some things I want to mention: 1) Static references (tightly coupled code). I had replaced it with DI. 2) Strange way to work with exceptions. I've removed exception declarations from method signatures, and replaced some checked exceptions with runtime ones. 3) Guava methods to create collections/maps, such as Maps.newHashMap() makes code cleaner. 4) Some big methods had been split into small parts. 5) Executor and job factory classes has been extracted. For Fjodor Vershinin , you can try to replace non type-safe code for getting query parameters with JAX-RS. (Java API for restful services). This can give two benefits: type-safety and code reuse (you can reuse jax-rs models in your client, where proxy can be generated automatically).
          Hide
          fjodor.vershinin Fjodor Vershinin added a comment -

          Yes, I agree, JAX-RS looks very promising. We need to take this issue into GSOC scope I guess.

          Show
          fjodor.vershinin Fjodor Vershinin added a comment - Yes, I agree, JAX-RS looks very promising. We need to take this issue into GSOC scope I guess.
          Hide
          lewismc Lewis John McGibbney added a comment -

          Hi Ivan Vershinin, apologies I did not see your patch here. This is a valuable contribution so thank you. It will really help with pushing on GSoC this year.
          RE: JAX-RS, YES I am a big fan and it will tie in nicely with the eventual Wicket-based UI Fjodor Vershinin will be working on.
          Ivan Vershinin, I will not be looking in to this patch until we begin formal work on the GSoC project (around 2-3 weeks or so) but I have not forgotten about it. Thank you again for the patch.

          Show
          lewismc Lewis John McGibbney added a comment - Hi Ivan Vershinin , apologies I did not see your patch here. This is a valuable contribution so thank you. It will really help with pushing on GSoC this year. RE: JAX-RS, YES I am a big fan and it will tie in nicely with the eventual Wicket-based UI Fjodor Vershinin will be working on. Ivan Vershinin , I will not be looking in to this patch until we begin formal work on the GSoC project (around 2-3 weeks or so) but I have not forgotten about it. Thank you again for the patch.
          Hide
          fjodor.vershinin Fjodor Vershinin added a comment -

          I've done migration to JAX-RS, but new API is not compatible with old one.
          I think, it was good place to start for me, because now I better understand how it should work.
          This patch contains both Ivan Vershinin's refactorings and my JAX-RS stuff.

          Show
          fjodor.vershinin Fjodor Vershinin added a comment - I've done migration to JAX-RS, but new API is not compatible with old one. I think, it was good place to start for me, because now I better understand how it should work. This patch contains both Ivan Vershinin 's refactorings and my JAX-RS stuff.
          Hide
          lewismc Lewis John McGibbney added a comment -

          This looks dynamite. I am unsure whether to branch the Nutch 2.X codebase and we can work directly with that, or else work on your Guthub branch or something similar.
          Do you have any preferences?

          Show
          lewismc Lewis John McGibbney added a comment - This looks dynamite. I am unsure whether to branch the Nutch 2.X codebase and we can work directly with that, or else work on your Guthub branch or something similar. Do you have any preferences?
          Hide
          fjodor.vershinin Fjodor Vershinin added a comment -

          I would prefer to work on 2.x codebase, because there will be less conflicts. For example, after migration to gora 4.0, there were changes in API classes and it was required to resolve them.

          Show
          fjodor.vershinin Fjodor Vershinin added a comment - I would prefer to work on 2.x codebase, because there will be less conflicts. For example, after migration to gora 4.0, there were changes in API classes and it was required to resolve them.
          Hide
          lewismc Lewis John McGibbney added a comment -

          Hi Fjodor Vershinin, can you please attach an SVN-friendly version of this patch?
          git diff --no-prefix trunk > NUTCH-1769.patch
          Thank you

          Show
          lewismc Lewis John McGibbney added a comment - Hi Fjodor Vershinin , can you please attach an SVN-friendly version of this patch? git diff --no-prefix trunk > NUTCH-1769 .patch Thank you
          Hide
          fjodor.vershinin Fjodor Vershinin added a comment -

          Ok, this is updated version

          Show
          fjodor.vershinin Fjodor Vershinin added a comment - Ok, this is updated version
          Hide
          lewismc Lewis John McGibbney added a comment -

          Committed revision 1605644 in 2.X HEAD
          Thank you Fjodor Vershinin for the patch. I've run it through the motions based on your documentation [0] and it is standing up well.

          [0] https://wiki.apache.org/nutch/NutchRESTAPI

          Show
          lewismc Lewis John McGibbney added a comment - Committed revision 1605644 in 2.X HEAD Thank you Fjodor Vershinin for the patch. I've run it through the motions based on your documentation [0] and it is standing up well. [0] https://wiki.apache.org/nutch/NutchRESTAPI
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Nutch-nutchgora #1060 (See https://builds.apache.org/job/Nutch-nutchgora/1060/)
          NUTCH-1769 REST API refactoring (lewismc: http://svn.apache.org/viewvc/nutch/branches/2.x/?view=rev&rev=1605644)

          • /nutch/branches/2.x/CHANGES.txt
          • /nutch/branches/2.x/ivy/ivy.xml
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/APIInfoResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/AdminResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/ConfManager.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/ConfResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/DbReader.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/DbResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/JobManager.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/JobResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/JobStatus.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/NutchApp.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/NutchServer.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/Params.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/JobFactory.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/JobWorker.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/NutchServerPoolExecutor.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/RAMConfManager.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/RAMJobManager.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbIterator.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbPageConverter.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbReader.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/misc
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/misc/ErrorStatusService.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/DbFilter.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/JobConfig.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/NutchConfig.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/DbQueryResult.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/ErrorResponse.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/JobInfo.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/NutchStatus.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/AbstractResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/AdminResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/ConfigResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/DbResource.java
          • /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/JobResource.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Nutch-nutchgora #1060 (See https://builds.apache.org/job/Nutch-nutchgora/1060/ ) NUTCH-1769 REST API refactoring (lewismc: http://svn.apache.org/viewvc/nutch/branches/2.x/?view=rev&rev=1605644 ) /nutch/branches/2.x/CHANGES.txt /nutch/branches/2.x/ivy/ivy.xml /nutch/branches/2.x/src/java/org/apache/nutch/api/APIInfoResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/AdminResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/ConfManager.java /nutch/branches/2.x/src/java/org/apache/nutch/api/ConfResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/DbReader.java /nutch/branches/2.x/src/java/org/apache/nutch/api/DbResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/JobManager.java /nutch/branches/2.x/src/java/org/apache/nutch/api/JobResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/JobStatus.java /nutch/branches/2.x/src/java/org/apache/nutch/api/NutchApp.java /nutch/branches/2.x/src/java/org/apache/nutch/api/NutchServer.java /nutch/branches/2.x/src/java/org/apache/nutch/api/Params.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/JobFactory.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/JobWorker.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/NutchServerPoolExecutor.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/RAMConfManager.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/RAMJobManager.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbIterator.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbPageConverter.java /nutch/branches/2.x/src/java/org/apache/nutch/api/impl/db/DbReader.java /nutch/branches/2.x/src/java/org/apache/nutch/api/misc /nutch/branches/2.x/src/java/org/apache/nutch/api/misc/ErrorStatusService.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/DbFilter.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/JobConfig.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/request/NutchConfig.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/DbQueryResult.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/ErrorResponse.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/JobInfo.java /nutch/branches/2.x/src/java/org/apache/nutch/api/model/response/NutchStatus.java /nutch/branches/2.x/src/java/org/apache/nutch/api/resources /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/AbstractResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/AdminResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/ConfigResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/DbResource.java /nutch/branches/2.x/src/java/org/apache/nutch/api/resources/JobResource.java

            People

            • Assignee:
              fjodor.vershinin Fjodor Vershinin
              Reporter:
              vershinin Ivan Vershinin
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development