Reading-eScience-Centre / edal-java

Environmental Data Abstraction Layer libraries
Other
39 stars 30 forks source link

Allow getTimeseries download as text/json #73

Closed rsignell-usgs closed 7 years ago

rsignell-usgs commented 7 years ago

It would be nice to allow getTimeseries to have an INFO_FORMAT of text/json so that clients other than godiva3 could more easily access and plot the data (I'm thinking here of Terriajs...). It's more of a pain to parse text/xml.

jonblower commented 7 years ago

Yes, we should use CoverageJSON for this

rsignell-usgs commented 7 years ago

Uh oh, now CoverageJSON is out of the bag! :smile_cat:

guygriffiths commented 7 years ago

text/xml isn't a supported format for getTimeseries - are you thinking of text/csv?

Either way, I'll implement support for text/json and more specifically application/prs.coverage+json for both getTimeseries and getVerticalProfile and have them return CoverageJSON.

rsignell-usgs commented 7 years ago

@guygriffiths , oh, I guess that was the old ncWMS? Or maybe i was confused. In any case, it will be nice to have json! Thanks! And CoverageJSON looks super cool!

guygriffiths commented 7 years ago

OK, this is done in the develop branch.

rsignell-usgs commented 7 years ago

Is there an ETA for this functionality arriving in a ncWMS war file?

guygriffiths commented 7 years ago

Not at the moment, no. There's a bug in ncWMS 2.2.4 which occurs if a user wants to define a Tomcat context for ncWMS2 to tell it where to put the config directory. The Tomcat context overwrites the context included with ncWMS2 which defines how to handle reprojection (necessary since we started using Apache SIS in EDAL 1.2.4), and only a small handful of projections end up being supported.

We're currently discussing workarounds with a chap from Apache, but we can't do a release until this is fixed.

rsignell-usgs commented 7 years ago

Okay, thanks for the update and explanation.

rsignell-usgs commented 7 years ago

@guygriffiths , any update on the Apache issue?

guygriffiths commented 7 years ago

@rsignell-usgs No, not at the moment, sorry. I'll try and find some time this week to chase it up, but the majority of my time at the moment is taken up by other projects.

rsignell-usgs commented 7 years ago

@guygriffiths , fingers crossed you can find the time! The Unidata THREDDS Steering Team met last week and was discussing ncWMS2 features to be including in the upcoming TDS 5.0 release, and it would be great to have this in there.

guygriffiths commented 7 years ago

@rsignell-usgs I managed to find a (horribly hacky) workaround to the Apache issue and I've just done a release which contains these changes

rsignell-usgs commented 7 years ago

@guygriffiths , cool! I see the new ncWMS artifact here: https://github.com/Reading-eScience-Centre/ncwms/releases/download/ncwms-2.2.5/ncWMS2.war Thanks!

rsignell-usgs commented 7 years ago

Working!