monarch-initiative / phenogrid

The phenogrid widget
13 stars 14 forks source link

Uses a post for simsearch and get for the compare API function #124

Closed kshefchek closed 9 years ago

kshefchek commented 9 years ago

Configures the ajax call as a post for simsearch, but leaves the compare call as a get as this has not been configured as a post on the webapp side. See https://github.com/monarch-initiative/monarch-app/issues/720

harryhoch commented 9 years ago

@yuanzhou, please review and merge as appropriate...

yuanzhou commented 9 years ago

Testing now

yuanzhou commented 9 years ago

Two questions @kshefchek :

Q1

I did local (my uses 8282 for the monarch-app port) testing of http://localhost:8282/gene/NCBIGene:2048, and the requested URL for that POST is

http://localhost:8282/simsearch/phenotype?input_items=

and the HTTP post body turned out to be

input_items=HP:0012125+HP:0012125+HP:0012125+HP:0100006+HP:0100006+HP:0100006&target_species=10090&limit=10

So the input_items is repeated. Should you just remove it from the URL base?

Q2

When I was testing http://localhost:8282/gene/NCBIGene:100, I got this error:

GET http://localhost:8282/gene/NCBIGene:100/phenotype_list.json 500 (Java heap space)

Seems Java heap space in my local testing machine (CentOS 6.6 installed on VirtualBox) is not big enough. I could see the results if I go to http://beta.monarchinitiative.org/gene/NCBIGene:100/phenotype_list.json

Will this cause any configuration changes?

kshefchek commented 9 years ago

Q1 - sure we can remove the ?input_items part of the url base. Q2 - This is most likely unrelated to any of these changes. Sounds like a momentary issue with our beta solr server.

yuanzhou commented 9 years ago

Q1 - I got HTTP 500 error after having the ?input_items part removed from url base. And I did a REST test to the beta server, http://beta.monarchinitiative.org/simsearch/phenotype/ showed 500 error too with the same input

input_items=HP:0012125+HP:0012125+HP:0012125+HP:0100006+HP:0100006+HP:0100006&target_species=10090&limit=10

Do you need to make some changes in monarch-app/lib/monarch/web/webapp.js too?

Q2 - Increased the memory in my virtualbox and problem solved.

kshefchek commented 9 years ago

In testing locally I'm not having any issues, will investigate.

yuanzhou commented 9 years ago

@kshefchek try this chrome REST app: https://chrome.google.com/webstore/detail/resteasy/nojelkgnnpdmhpankkiikipkmhgafoch

only

http://beta.monarchinitiative.org/simsearch/phenotype?input_items=

works,

http://beta.monarchinitiative.org/simsearch/phenotype

doesn't work.

With the same POST body

input_items=HP:0012125+HP:0012125+HP:0012125+HP:0100006+HP:0100006+HP:0100006&target_species=10090&limit=10
kshefchek commented 9 years ago

The issue might be the final forward slash.

Sent from my iPhone

On Jul 29, 2015, at 1:52 PM, yuanzhou notifications@github.com wrote:

Q1 - I got HTTP 500 error after having the ?input_items part removed from url base. And I did a REST test to the beta server, http://beta.monarchinitiative.org/simsearch/phenotype/ showed 500 error too with the same input

input_items=HP:0012125+HP:0012125+HP:0012125+HP:0100006+HP:0100006+HP:0100006&target_species=10090&limit=10 Do you need to make some changes in monarch-app/lib/monarch/web/webapp.js too?

Q2 - Increased the memory in my virtualbox and problem solved.

— Reply to this email directly or view it on GitHub.

yuanzhou commented 9 years ago

I tested it again with the REST app, the forward slash didn't make any difference. Not sure if it's an issue with that chrome REST app. @kshefchek , can you please rebase the code and update the PR so I can auto merge it?

yuanzhou commented 9 years ago

@kshefchek no worries about this PR, I'll close this PR and apply your changes.