Conversation
|
also adding @salichon for awareness |
junghao
left a comment
junghao
left a comment
There was a problem hiding this comment.
Could you please add the test cases to the routes test, see how querying other events and volcano-tectonic parameter works?
Thanks.
| var nslcReg = regexp.MustCompile(`^([\w*?,]+|--)$`) | ||
| var eventTypeReg = regexp.MustCompile(`^([\w*?, ]+|--)$`) // space allowed | ||
| // nslcReg: FDSN spec allows all ascii, but we'll only allow alpha, number, _,-, ?, *, "," and "--" (exactly 2 hyphens only) | ||
| var nslcReg = regexp.MustCompile(`^([\w*?,]+(?:-[\w*?,]+)*|--)$`) // space not allowed |
There was a problem hiding this comment.
This nslcReg is used to verify network/station/location/channel identifiers, is not for event type.
We don't have any identifiers containing -.
However, it's fine to add this since it's in FDSN specification.
| var eventTypeReg = regexp.MustCompile(`^([\w*?, ]+|--)$`) // space allowed | ||
| // nslcReg: FDSN spec allows all ascii, but we'll only allow alpha, number, _,-, ?, *, "," and "--" (exactly 2 hyphens only) | ||
| var nslcReg = regexp.MustCompile(`^([\w*?,]+(?:-[\w*?,]+)*|--)$`) // space not allowed | ||
| var eventTypeReg = regexp.MustCompile(`^([\w*?, ]+(?:[ -][\w*?,]+)*|--)$`) // space allowed |
There was a problem hiding this comment.
Not sure which cases are this change aim to deal with?
If it's for -, then I think adding a \- to existing regex is sufficient.
^([\w*?,\- ]+|--)$
There was a problem hiding this comment.
This is to allow - in the middle, and it needs to be multiple times, yes adding a - to existing regex is simpler, may be it doesn't matter if - is in the start, but it also allow more than 2 hyphens (---)
There was a problem hiding this comment.
Unless the specification says "no hyphen at the start" (though people won't do it) otherwise we don't need to make sure it's in the middle.
bpeng
force-pushed
the
seiscomp-update
branch
11 times, most recently
from
4a0469d to
5b6516e
Compare
add routes test for event types as per review
|
@bpeng Update event type in validation and add tests for new event types I am interested to see an example this looks good as a process thanks |
salichon
left a comment
salichon
left a comment
There was a problem hiding this comment.
with regards to check with examples :)
|
Thanks @salichon
The additional types should be already working for scml processing and the event types can be added to the database, but I haven't seen any events with the volcanic types in The updates in this PR is to allow queries by the new event types like the following (currently it returns 400 error) after deployment (probably get 204 response if there is not such event with the specified type in the DB). |
5 participants
Ticket https://github.com/GeoNet/tickets/issues/17374
Update event type in validation and add tests for new event types
Deployed on dev, try