EOL / tramea

A lightweight server for denormalized EOL data
Other
2 stars 1 forks source link

[5] Media json response #148

Closed jhammock closed 8 years ago

jhammock commented 8 years ago

Just like https://github.com/EOL/tramea/issues/39, but for media, per request of @hyanwong

AmrMMorad commented 8 years ago

Done, merged to the master. It is now ready for next deploy.

jhammock commented 8 years ago

Sorry for the delayed update. Post-deploy, I can't get an example url for this to work, but I suspect I don't know how to construct one. @AmrMMorad , could you provide one for @hyanwong?

AmrMMorad commented 8 years ago

Sure, As you know, previously API used to have one parameter to express the media stuff (such as images, videos). Now we have 2 parameters express that: one for the page (which page do you want to get) and the other controls how many media items can be shown within a page. So if we need image 5, we need to adjust (for example) images_page = 4 and images_per_page = 2 (or any other valid combination), so it will return the second 4 images (starting from 5 to 8) and etc. an example for this as follows: http://eol.org/api/pages/1.0.json?batch=false&id=1045608&images_per_page=75&images_page=2&videos_per_page=0&videos_page=1&sounds_per_page=0&sounds_page=1&maps_per_page=0&maps_page=1&texts_per_page=2&texts_page=1&iucn=false&subjects=overview&licenses=all&details=true&common_names=true&synonyms=true&references=true&taxonomy=true&vetted=0&cache_ttl=&language=en Could you please let me know if you have any problem? Thanks

jhammock commented 8 years ago

Some API users have a back compatibility problem with this change; let's discuss at next tech discussion meeting.

hyanwong commented 8 years ago

I can use it, but I don't think it is backwards compatible.

Whatever you do, can you at least allow it to be used as present. I suggest that you simply allow 'images' as a synonym of 'images_per_page', and so on for videos etc. This should retain backwards compatibility

AmrMMorad commented 8 years ago

Done. Both images and images_per_page (and etc..) are equivalent. I will merge this to the master and after the deployment, it is expected to be solved. Thanks

jhammock commented 8 years ago

Deployed; @hyanwong @kueda please advise if there are any further issues. Thanks, all

hyanwong commented 8 years ago

Quick work. Thanks

KatjaSchulz commented 8 years ago

I am experiencing a problem with the collections API that may be related to this. When I try to access the items from a particular collection, I cannot get past page 1 of the API results, i.e., http://eol.org/api/collections/1.0/122675.xml?page=2&per_page=500&filter=&sort_by=alphabetical&sort_field=&cache_ttl=&language=en gets me exactly the same results as http://eol.org/api/collections/1.0/122675.xml?page=1&per_page=500&filter=&sort_by=alphabetical&sort_field=&cache_ttl=&language=en

The same happens when I ask for the json response. It looks like the page parameter is ignored.

jhammock commented 8 years ago

I took a look at the results of the example in Amr's comment above, and I see one puzzling thing that may or may not be related to pagination. The result includes both sounds and video objects, although sounds_per_page=0 and videos_per_page=0. If I set sounds_page=0 or videos_page=0, that content disappears. That behavior is not a problem, I suppose; I mention it only because I wondered if there was confusion between the _page and _per_page parameters.

All other behavior incrementing _page or _per_page parameters seems as expected. It is only that _per_page=0 behaves like _per_page=1, (this is also the default behavior if a value is not specified), and only _page=0 prevents any content in a category from displaying.

jhammock commented 8 years ago

If these new symptoms belong in https://github.com/EOL/tramea/issues/39, or a new ticket, please comment and close

AmrMMorad commented 8 years ago

@jhammock In fact I was, intentionally, doing so. I was overwriting the value with 1 if the user puts any value less than 1 in this param. I did a fix in a separate branch for now which only overwrites the value with 1 if the value isn't given; otherwise it will be taken as is. Is that ok? Shall I push this to the master?

AmrMMorad commented 8 years ago

Regarding the collections part, I guess we need to open a new bug and relate this to these tickets. When I tried the example that Jeremy added in #39, it actually work.

jhammock commented 8 years ago

Thanks, Amr! Yes, if the user doesn't explicitly enter a value, a default of 1 sounds good. It sounds to me like that's ready to go. I'll create a new ticket for the collections pagination issue.

AmrMMorad commented 8 years ago

Thanks, Jen. I've merged this change to the master now. Thanks!

kueda commented 8 years ago

Not entirely sure why I was mentioned here. I emailed EOL a few weeks ago about URLs like http://eol.org/api/pages/1.0/1052070.xml?text=0&images=0&sounds=0&videos=0&maps=50&subjects=all&details=true not filtering by content type anymore. They still don't, presuably because you guys changed the way content filtering and pagination works with the page API. My original questions were:

  1. Have you guys permanently changed the page API?
  2. If so, why did you make a reverse-incompatible change to your API without changing the API version?
  3. If you're going to change your API without changing the version, how do I get updates about these changes so I can prevent iNaturalist's EOL integration from breaking?

I still don't have answers to any of these questions, but maybe this isn't the right place to ask them. Does this belong in http://eol.org/communities/121/newsfeed or something?

hyanwong commented 8 years ago

@kueda - I'm not an EoL person but 1) Yes, changed 2) That was a mistake - it should have been backwards compatible. I have suggested an improvement to re-introduce backwards compatibility. So does it still not work? Damn. 3) For some reason there is some reluctance to change the API version number. But hopefully future changes will take this into account, or introduce a major/minor number system, or something.

jhammock commented 8 years ago

Thanks, @hyanwong ; sorry, @kueda ! There is a remaining fix for the back compatibility that is not yet deployed. @AmrMMorad , could you please identify the branch with the fix allowing the zero values? The next full deploy may be a little while, but perhaps we can expedite this bit.

AmrMMorad commented 8 years ago

@jhammock : it has been merged to the master; waiting for the next deployment. Thanks

hyanwong commented 8 years ago

I'm hitting this problem now. Presumably not yet merged. I can't figure out any way at the moment to get e.g. no texts returned. E.g. http://eol.org/api/pages/1.0.json?batch=false&id=1045608&images_per_page=1&images_page=1&videos_per_page=0&videos_page=0&sounds_per_page=0&sounds_page=0&maps_per_page=0&maps_page=0&texts_per_page=0&texts_page=0&iucn=false&subjects=overview&licenses=pd%7Ccc-by%7Ccc-by-sa&details=true&common_names=true&synonyms=false&references=false&taxonomy=false&vetted=2&cache_ttl=&language=en

Still gives a text object, although it's only images I specified. Will this work on next deploy?

JRice commented 8 years ago

...I think we just deployed a fix for this.

kueda commented 8 years ago

http://eol.org/api/pages/1.0/1052070.xml?text=0&images=0&sounds=0&videos=0&maps=50&subjects=all&details=true still seems to be returning things that aren't maps, e.g. SillImage. Is there some kind of cache that needs to be cleared?

AmrMMorad commented 8 years ago

In the result of this example, the data subtype is map so in fact they are considered maps in eol (eol solr which we query to get the data). So these objects are considered maps not images. Thanks

kueda commented 8 years ago

My mistake. However, I am still seeing the following in the response to http://eol.org/api/pages/1.0/1052070.xml?text=0&images=0&sounds=0&videos=0&maps=50&subjects=all&details=true:

<dataObject>
<dataObjectID>e1d0ae70720f012f69d0586ebfed309c</dataObjectID>
<taxonConceptID>1052070</taxonConceptID>
<dataType>http://purl.org/dc/dcmitype/Text</dataType>
<mimeType>text/plain</mimeType>
<agent homepage="" role="author">Leo Shapiro</agent>
<agent homepage="" role="provider">Leo Shapiro</agent>
<dcterms:created>2012-04-26 21:21:20 UTC</dcterms:created>
<dcterms:modified>2012-04-26 21:21:20 UTC</dcterms:modified>
<dc:language>en</dc:language>
<license>http://creativecommons.org/licenses/by/3.0/</license>
<dcterms:rightsHolder>Leo Shapiro</dcterms:rightsHolder>
<subject>
http://rs.tdwg.org/ontology/voc/SPMInfoItems#TaxonBiology
</subject>
<dc:description>
The loud, clear songs of the Northern Cardinal (<i>Cardinalis cardinalis</i>) are a familiar part of the suburban soundscape across the eastern United States. The Northern Cardinal is abundant throughout the eastern United States and adjacent Canada, with a range extending south to Belize. This species has also been introduced to the Hawaiian Islands, where it is well established on all the main islands from Kauai eastward, and has been established locally in coastal southern California and in Bermuda. These striking birds inhabit woodland edges, swamps, streamside thickets, and suburban gardens, as well as the Sonoran Desert and riparian areas of the Southwest. In the East, the Northern Cardinal has expanded its range northward during the past century. Northern Cardinals are permanent residents throughout their range. The Northern Cardinal has been selected as the state bird of Illinois, Indiana, Kentucky, North Carolina, Ohio, Virginia, and West Virginia. The diet of the Northern Cardinals is highly varied, but consists mainly of seeds, insects, and berries. The young are fed mostly insects. The male sings to defend his nesting territory and actively attacks intruders. In courtship, both male and female raise their heads high and sway back and forth while singing softly. Early in the breeding season, the male often feeds the female. The female sings mainly in the spring before nesting. The nest, which is an open cup built by the female, is typically hidden in dense vegetation 1 to 3 m above the ground, sometimes higher. The 3 to 4 (sometimes 2 or 5) eggs (whitish to pale bluish or greenish white marked with brown, purple, and gray) are incubated for 12 to 13 days, almost always by the female alone. Both parents feed the young, which leave the nest around 9 to 11 days after hatching. The male may continue to feed the fledglings as the female initiates a second brood. There may be two to three broods per year (rarely four). (Kaufman 1996; AOU 1998; Dunn and Alderfer 2011)
</dc:description>
<additionalInformation>
<vettedStatus>Trusted</vettedStatus>
<dataRating>2.5</dataRating>
<dataObjectVersionID>18376500</dataObjectVersionID>
</additionalInformation>
</dataObject>

That does not seem to have a subtype of map.

jhammock commented 8 years ago

@kueda you want "texts=0", not "text=0"

kueda commented 8 years ago

Got it. FWIW, I'm pretty sure the API param used to be text=0, as that's what we have been using at iNat before the change. Maybe you guys used to support both.

jhammock commented 8 years ago

and it's not a bad synonym to support anyway...