Skip to content

Conversation

@anguss00
Copy link
Contributor

@anguss00 anguss00 commented Jan 8, 2018

Requests are validated as follows:

Subset, layer and callback email are required parameters.
Subset must have a latitude, longitude and temporal section.
If these conditions are not met, the request aborts before going to batch.

Add geonetwork URL as sandbox for development mode.

@jonescc
Copy link
Contributor

jonescc commented Jan 8, 2018

@anguss00 - time should not be a required parameter - we just made it optional not long ago to support bathymetry files which don't have a temporal dimension.

@jonescc
Copy link
Contributor

jonescc commented Jan 8, 2018

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 8, 2018

Gotcha, thanks @jonescc. Will update

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 8, 2018

Probably better to validate on the regex defined in GGD originally. Will update this

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 9, 2018

Done @jonescc

throw new ValidationException("Request must have a subset");
} else {
int latLonCount = 0;
Pattern latLonPattern = Pattern.compile("([+-]?\\d+\\.?\\d+)\\s*,\\s*([+-]?\\d+\\.?\\d+)");
Copy link
Contributor

@jonescc jonescc Jan 10, 2018

Choose a reason for hiding this comment

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

Should probably align better with aggregator validation https://github.com/aodn/aws-wps/blob/master/aggregation-worker/src/main/java/au/org/emii/geoserver/client/SubsetParameters.java#L40 - we haven't used regexes for some time - they were problematic

Copy link
Contributor

Choose a reason for hiding this comment

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

Even better if they used the same code so they couldn't get out of synch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a really good point. I'll amend that

@gsatimos
Copy link
Contributor

@anguss00 is this still a pending merge?

@anguss00
Copy link
Contributor Author

This was an extra reserve task that just didn't quite make it in. So yes, pending merge, but I think needs another change or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants