Repository navigation
Fix Cone Search decoding and timeout test - #4757
Conversation
… result (side effect of astropy#4699) when it is not supposed to. Not sure why I did not catch this earlier.
|
p.s. As with my other Cone Search fixes, I'll merge by the end of the week (COB Friday, Eastern Time) unless I hear otherwise. |
|
@pllim - this looks good to me. If a regression test is easy to add, that would be great. Also, I don't see a reason not to backport -- both these would seem bona-fide bugs (cc @eteq, @astrofrog). |
|
@mhvk , if this is backported, #4699 needs to be too. No one ever runs the validation but me. I don't see the point to cause extra pain to the backporter. But, that is up to you and other core maintainers to decide. I don't want to add yet another test just to test this thing, but maybe I can somehow modify an existing test to accommodate this. |
|
@mhvk , good news about the tests. Actually, this is already caught by existing tests using: >>> astropy.test('vo', remote_data=True)Log attached: To clarify again, this PR fixes both of those failed tests. |
|
OK, great the test is there! Since this is something that was triggered by external services changing their behaviour, it would seem to make sense to backport, but since I'm not the one doing the backporting, I'll let this be decided by someone who does... |
|
@mhvk , just to clarify somemore, this does not directly affect Cone Search behavior on user's side, but mostly the validation side (which I don't think anyone does but me) that provides a default list for the Cone Search. Most use cases that I have come across, the users already know which Cone Search services they want, so I don't think they blindly use the default list anyway. Or, to put this another way, if an user uses the stable version of Cone Search today, he/she still sees a change in the default list, because this default list is generated nightly by STScI (i.e., me) and put on a server at STScI that Cone Search accesses. So, I don't think there is a need to backport. Does this make sense? |
|
Yes, this does make sense. And since this is all OK, I'll merge now... |
|
Thanks, @mhvk ! |
|
Re-milestoning this since it's needed to fix the error in 1.0.10 too, and applies cleanly |
Fix Cone Search decoding and timeout test
Several days ago, a new Cone Search service must have been added to the STScI database. It contains an entry that breaks
bytestring.decode('ascii'). The solution is to usebytestring.decode('utf-8')instead.Minimal VO table to reproduce the error is as follows (rename the
.txtextension to.xml):VOTshort.txt
Specifically, the offending line is its title, as illustrated below:
Also in this PR is a fix for
test_timeout()that specifically tests that Cone Search reports timeouts appropriately. The test must have been broken by a fix to the caching system in #4699, although I am unsure how it wasn't caught back then.As Cone Search is not a widely used feature (i.e., no one complains about it but me), I am okay with putting the fix to the next release instead of backporting.