Relaxed baseurl requirements in SIA2Service #500
Conversation
|
On Tue, Nov 28, 2023 at 02:30:10PM -0800, Adrian wrote:
Solution for #450.
I'm afraid not quite yet, although it now at least works with DaCHS
services.
Try this:
import time
import pyvo
import requests
class SessionWithTimeout(requests.Session):
def request(self, *args, **kwargs):
if "timeout" not in kwargs:
kwargs["timeout"] = 3
return super().request(*args, **kwargs)
requests.Session = SessionWithTimeout
for rec in pyvo.registry.search(servicetype="sia2"):
try:
_ = rec.service.search(maxrec=1)
print(f"ok: {rec.ivoid}")
except KeyboardInterrupt:
print(f"hung: {rec.ivoid}")
time.sleep(0.2)
except Exception as exc:
print(f"broken: {rec.ivoid} ({exc})")
This tries all the registered SIA2 services, and the tally is that 91
are broken even after this PR (including one that shouldn't have
published their SIA2 service because they have none; I'll take care),
and 11 work.
Given that capabilities have absolutely no function for SIA2 outside
of auth (for which we don't have a standard), I'd say we shouldn't
insist on the endpoint when this breaks the large majority of
services that actually exist in the registry.
We simply are not a validator (and I note in passing that I think
what validator exists doesn't look at the capabilities either yet).
|
|
That first attempt was wrong indeed. Here is another one in which If |
There was a problem hiding this comment.
The current tally of my all-VO tester has the following broken services:
- ivo://au.csiro/casda/sia2 -- that's geofencing, I think
- ivo://esavo/hst/siap -- needs investigation: I got a 503 from them, so it's probably not our fault
- Everything from ipac -- again, needs investigation; these are read timeouts. Hu?
- ivo://ned.ipac/sia -- that's a 500; we should look into it, but that's not a showstopper
- ivo://pds-ppi/system/siap2/sitewide -- that's accidentally published and should vanish soon.
So... what's left in terms of failures looks like it's not pyVO's fault. If you understand the remaining CI failures (there are ~ two in general pyVO code ATM), I'd say let's go.
weird, I'll have a look at this or propagate it to the right persons internally. |
|
@msdemlei - could you please go into more details about the queries you try and see for IRSA? E.g. the following one works for me, as well as the one in #450 : |
The CI for the |
| @@ -156,7 +156,7 @@ class SIA2Service(DALService, AvailabilityMixin, CapabilityMixin): | |||
| generally not notice that, though. | |||
| """ | |||
|
|
|||
| def __init__(self, baseurl, session=None): | |||
| def __init__(self, baseurl, session=None, check_baseurl=True): | |||
There was a problem hiding this comment.
Seaparate, but I would like to make optional arguments keyword only, systematically, and before the 1.5 release.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #500 +/- ##
=======================================
Coverage 80.08% 80.08%
=======================================
Files 52 52
Lines 6071 6077 +6
=======================================
+ Hits 4862 4867 +5
- Misses 1209 1210 +1 ☔ View full report in Codecov by Sentry. |
|
@bsipocz - OK now? |
|
Awesome. Appreciated @bsipocz |
|
On Wed, Nov 29, 2023 at 11:31:17AM -0800, Adrian wrote:
If `capabilities` is not to be used, then this should also be
pushed to the standard. @msdemlei do you mind opening an issue if
there isn't one already? I would, except that I don't know how to
justify it since to me that end point makes perfect sense.
I'm not saying it doesn't make sense at all; for instance, it'd be
great if web interfaces for registering services would take the
capabilities element from such an endpoint. I'm just saying *we* (in
pyVO) don't need it and hence shouldn't fail if it's not there.
Failing on spec violations the job of a validator, not of a client
("be strict in what you produce, lenient in what you accept").
Having said that, the requirement levels on the various VOSI
endpoints have been a constant source of pain ever since VOSI itself
said availability and capabilities are mandatory without saying how
to get from a world where no service had VOSI endpoints to its
desired end state -- without giving too much of a reason for the
requirements on top.
What we really should do: Update VOSI to drop availability. But
that's a different topic.
|
|
On Fri, Dec 01, 2023 at 10:03:32AM -0800, Brigitta Sipőcz wrote:
@msdemlei - could you please go into more details about the queries
you try and see for IRSA? E.g. the following one works for me, as
well as the one in #450 :
Ah... now that I look again... these are ReadTimeouts; in my script
(above), I'm timing out after three seconds, so if IRSA takes a while
to respond, they'll register as bad.
Raising the timeout to 8 seconds makes most of the IRSA services
pass; the others still have ReadTimeouts. Frankly, I wonder why these
would take so long to return a MAXREC=1 query, but it's pretty clear
that pyVO isn't at fault.
|
Solution for #450.
Still not sure that it's correct to relax the adherence to the spec in order to accommodate services that don't, but here you have it.
SIA2 was not wrong in what it was doing, just that it didn't expect the access URL to contain a query. It can deal with that now. Note that the service is still required to have a
capabilitiesend point.