Skip to content
This repository has been archived by the owner on Oct 20, 2018. It is now read-only.

Cleaning Rest Module #312

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

dav009
Copy link
Member

@dav009 dav009 commented Aug 27, 2014

This is an attempt on cleaning the rest module:

  • Tried to get rid of repeated code/methods follwing DRY as much as I could
  • confidence param refers to disambiguationConfidence
  • added a new parameter spotterConfidence
  • Got rid of spotlightInterface
  • Server has a SpotlightModel
  • Endpoints call methods of spotlight model
    • spot calls spot
    • annotate calls firstBest
    • candidates calls nBest
  • I couldnt see any difference between disambiguate and annotate so I merged both endpoints in the same jersey resource
  • Resources only gather parameters and send them to spotlightModel and then to the output manager to get the correct output, this resulted in lots of logic being removed from server.java and the resources themselves.

ToDo:

  • Properly test this branch.
    • Test Json, Xml request
    • Test form requests

Other nice things to do ( probably in another PR):

  • add swagger
  • bump to jersey 2.x
  • replace @queryparams for @beanparam

  • It would be nice to update to jersey 2.x in such way we could create a bean with all the params fields and avoid naming them over and over in every endpoint. The problem is when I tried to bump jersey version I found errors in other modules (uima..) and this was already a big PR.

@dav009 dav009 changed the title Cleaning the rest endpoint Cleaning Rest Module Aug 28, 2014
@tgalery tgalery mentioned this pull request Sep 3, 2014
@tgalery
Copy link
Member

tgalery commented Sep 30, 2014

I think it would be good to squash some of the commits a bit. I was also thinking that keep a disambiguate endpoint, which takes a surface form, and a context as input. In this way, we could integrate more easily with other spotters (say you extract subject and object via a relationship extraction mechanism or what not).

@dav009
Copy link
Member Author

dav009 commented Sep 30, 2014

I will squash most of those commits.

About the disambiguate endpoint, in the master branch is just a copy/paste of the annotate endpoint doing exactly the same thing, so it is still reachable here but it reuses the code of annotate.
So the disambiguate endpoint you mention could be a batch of future work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants