Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DAP storage option #513

Closed
wants to merge 2 commits into from
Closed

Add DAP storage option #513

wants to merge 2 commits into from

Conversation

huard
Copy link
Collaborator

@huard huard commented Feb 7, 2020

Overview

Fix #512 (in theory). Untested in practice, will need a DAP server running alongside a WPS server to test.

Additional Information

Contribution Agreement

(as per https://github.com/geopython/pywps/blob/master/CONTRIBUTING.rst#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to PyWPS. I confirm that my contributions to PyWPS will be compatible with the PyWPS license guidelines at the time of contribution.
  • I have already previously agreed to the PyWPS Contributions and Licensing Guidelines

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 75.687% when pulling 069d29f on bird-house:fix-512 into cbca3cf on geopython:master.

@jachym
Copy link
Member

jachym commented Feb 9, 2020

@huard I think, the PR is great improvement - even I myself do not use the DAP storage feature at the moment, I love the interoperability possibilities, it offers.

Only thing I'm missing in your PR is update of documentation - you have to add section to configuration AND I would love to see a chapter in extensions too.

Is it possible to add some tests as well?

@huard
Copy link
Collaborator Author

huard commented Feb 10, 2020

Yes, I'll try to launch a dap server with emu to test it's really working.

Ideally, we'd have a general solution for this. Imagine for example that a process creates a shapefile and you'd like to serve it using wms and wcs. What should the wps response be ?

@jachym
Copy link
Member

jachym commented Feb 10, 2020

Hi,

this is one of the dream features, which used to be in PyWPS, but had to go due to lack of resources.

Anyway, IMHO, this should be steered by mimetype - if the mimetype is image/tiff, it can be still link to wms or wcs, but the result should be the file. If the result is the service, then the mimetype should be application/vnd.ogc.wms_xml and the response should be the capabilities document IMHO.

@cehbrecht cehbrecht added this to the 5.0.0 milestone Feb 10, 2020
Copy link
Collaborator

@cehbrecht cehbrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments.

@@ -201,7 +202,10 @@ def _json_reference(self, data):
if self.prop == 'url':
data["href"] = self.url
elif self.prop is not None:
self.storage = FileStorageBuilder().build()
if self.data_format.mime_type == "application/x-ogc-dods":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not check the configured storagetype ... whenever I have a dods file it will use the DapStorageBuilder even when it is not configured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

@cehbrecht
Copy link
Collaborator

@huard I think the DapStorage type behaves different then the existing ones. For the existing ones you only use the existing storage type: file or S3. With DAP it applies only for opendap links. Other files (txt, json, ...) will still be returned via HTTP.

Is the DapStorage an extension of the FileStorage which decides on the mime type where to store and which output url to return?

@huard
Copy link
Collaborator Author

huard commented Feb 10, 2020

yes, that was the idea.
Should everything be served by dap? Or should it be configurable?

@huard
Copy link
Collaborator Author

huard commented Feb 10, 2020

Thanks @cehbrecht for your comments. I realize it was a pretty naive implementation.

I just realized that in the case where we want everything to be stored on a DAP server, we only have to modify the FileStorage configuration. I suggest to close this PR for now and see if we can do everything we need with the current FileStorage implementation and a tweaked configuration.

@cehbrecht
Copy link
Collaborator

cehbrecht commented Feb 10, 2020

yes, that was the idea.
Should everything be served by dap? Or should it be configurable?

I don't think we want to serve everything by DAP. I'm not sure if OpenDAP (and WMS, WCS, ...) is a storage type. Maybe an output service type (which we do not have yet)? The services may still use the same storage: file, S3, ... Depending on the mimetype and the configuration the output urls would be adapted to HTTP, OpenDAP, WMS, ...

@jachym what would you think?

@huard
Copy link
Collaborator Author

huard commented Feb 10, 2020

Something like:

CONFIG.add_section('service')
CONFIG.set("application/x-ogc-dods", "{host}/dap/dodsC/outputs")

and then the storage.url method uses this configuration to override the "file" url if needed ?

@cehbrecht
Copy link
Collaborator

Something like:

CONFIG.add_section('service')
CONFIG.set("application/x-ogc-dods", "{host}/dap/dodsC/outputs")

and then the storage.url method uses this configuration to override the "file" url if needed ?

or maybe like this:

CONFIG.add_section('service')
CONFIG.set("dap_url", "{host}/dap/dodsC/outputs")
CONFIG.set("wms_url", "{host}/wms/outputs")

PyWPS may know the mimetypes.

@huard
Copy link
Collaborator Author

huard commented Feb 11, 2020

@cehbrecht I think explicit mime-types offer some advantages. You could have two instances of a WMS server, one for version 1.0 and another for version 2.0.

@cehbrecht
Copy link
Collaborator

@cehbrecht I think explicit mime-types offer some advantages. You could have two instances of a WMS server, one for version 1.0 and another for version 2.0.

Ok. But I suppose we need to configure it differently ... not using the mimetype as config parameter.

ping @jachym.

@huard
Copy link
Collaborator Author

huard commented Feb 11, 2020

Like the format name: 'GEOJSON, JSON, SHP, GML, METALINK, META4, KML, KMZ, GEOTIFF,'
'WCS, WCS100, WCS110, WCS20, WFS, WFS100,'
'WFS110, WFS20, WMS, WMS130, WMS110,'
'WMS100, TEXT, DODS, NETCDF, LAZ, LAS, ZIP,'
'XML'

@cehbrecht
Copy link
Collaborator

Like the format name: 'GEOJSON, JSON, SHP, GML, METALINK, META4, KML, KMZ, GEOTIFF,'
'WCS, WCS100, WCS110, WCS20, WFS, WFS100,'
'WFS110, WFS20, WMS, WMS130, WMS110,'
'WMS100, TEXT, DODS, NETCDF, LAZ, LAS, ZIP,'
'XML'

yes ... that could work.

@huard
Copy link
Collaborator Author

huard commented Feb 13, 2020

So will close this one and open another PR when I come up with a more general solution.

@huard huard closed this Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opendap support for outputs
4 participants