4306 Added notification for unsupported GeoServer projections. #4311
Conversation
https://sonarqube.cuahsi-workstation.com:9000/dashboard?id=hydroshare-4311 |
hs_file_types/models/geofeature.py
Outdated
| proj = osr.SpatialReference(wkt=str(self.originalcoverage.projection_string)) | ||
| authority = ':'.join((proj.GetAttrValue('AUTHORITY', 0), proj.GetAttrValue('AUTHORITY', 1),)) | ||
|
|
||
| with open(os.path.dirname(__file__) + '/../geoserver_epsg.txt') as f: |
There was a problem hiding this comment.
This method is the same implementation as raster. This approach also add an aggregation type specific method to the abstract metadata class that is used by all aggregation types. It can argued we already walked this path with preview_data_url, but I'd like to unwind this problem before we walk further down the path.
This implementation also reads and scans a file for every raster/feature aggregation in the resource each time the page is displayed. We should be doing this once and storing the result somewhere in the metadata so we're not reading the same file n times with each page load.
There was a problem hiding this comment.
Where would you recommend storing that type of metadata? I asked Alva about that when we first set up GeoServer and he said HydroShare didn't have a way to handle system metadata that isn't modifiable by users. That's why we went the route of generating the GeoServer URLs each time the page was loaded instead of doing it once and storing them somewhere.
There was a problem hiding this comment.
There is an extra_data field on each logical file for this type of thing. @hyi - used it for storing the URL in a .url file.
https://github.com/hydroshare/hydroshare/blob/develop/hs_file_types/models/base.py#L723
There was a problem hiding this comment.
I've moved the projection checks to the update_web_services task, so the check will only happen when web services are updated, and the result is stored in extra_data: 0c1176a
| 'If you want access to GeoServer features for those files, please make sure ' + | ||
| 'they are using a supported projection. ' + | ||
| '<br><strong><a ' + | ||
| 'href="' + geoserver_url + '/hydroshare/wms?request=GetCapabilities" ' + |
There was a problem hiding this comment.
A url path hardcoded into a javascript string of html is going to be difficult to manage updates if/when it changes.
There was a problem hiding this comment.
Moved GetCapabilities URL to settings.py: 28ca8e9
There was a problem hiding this comment.
I'm happy to talk about my concerns in more detail if you'd like. The biggest one is reading the file with each geofeature/raster aggregation. The other concerns are important, but there certainly is a precedent for the implementation you chose. But, I'd prefer to work out a more general solution with you.
hs_file_types/models/base.py
Outdated
| @@ -702,6 +702,12 @@ def get_preview_data_url(self, resource, folder_path): | |||
| # - subclass needs to override this | |||
| return None | |||
|
|
|||
| @classmethod | |||
There was a problem hiding this comment.
This method is no longer used and should be deleted
There was a problem hiding this comment.
I’ve removed this method here: 88b583f
| @@ -169,6 +171,8 @@ def data_store_structure(request): | |||
| resource=resource, | |||
| folder_path=f_store_path | |||
| ), | |||
| 'valid_geoserver_projection': | |||
| f.logical_file.extra_data.get('valid_geoserver_proj', 'True'), | |||
There was a problem hiding this comment.
Should this default to False?
Pull Request Checklist:
Positive Test Case