search

jdswinbank / Comet

A complete VOEvent transport system
http://comet.transientskp.org/
BSD 2-Clause "Simplified" License
23 stars 9 forks source link

'+' symbol in IVORN fragment? #37

Closed timstaley closed 8 years ago

timstaley commented 8 years ago

Hi John,

quick question on a technicality: currently IVORNS are given the yay/nay by this regex: https://github.com/jdswinbank/Comet/blob/62318be9e6d55aadddd579e1c3c40f2035e58d04/comet/utility/voevent.py#L20

Should the set of fragment characters include '+', or is there a good reason for exclusion?

Seems to be valid for HTTP, cf http://stackoverflow.com/a/1547940/725650, AFAICT, and it is occasionally useful when assigning substream IDs, e.g. if referring to some event generally known via its co-ordinates.

timstaley commented 8 years ago

Ok, the branch referenced above is the naive solution (trivial addition to the character list). Seems to work in practice but I'm guessing you may want to unit-test it.

jdswinbank commented 8 years ago

Thanks Tim -- I expect to be pretty much snowed under today, but I'll get to this as soon as I have a chance!

jdswinbank commented 8 years ago

I find the relevant IVOA recommendation pretty inscrutable here, unfortunately. After giving it a hard stare, I think I agree with you that this should be ok, and have implemented it in 9337eab51eaa. Could you confirm that that does what you want, please?

timstaley commented 8 years ago

Works great - many thanks!

jdswinbank commented 8 years ago

Thanks Tim. Have you tried the rest of the master branch? I'm going to push out a new Comet release with this included shortly, and any reports of success or failure would be of interest. There are quite a few changes, but mainly behind the scenes.

timstaley commented 8 years ago

Yes, I simply checked out the current master. All looks OK, have tested both comet-send and the usual 'listen and run handler' functionality.