astropy / pyvo

An Astropy affiliated package providing access to remote data and services of the Virtual Observatory (VO) using Python.
https://pyvo.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
74 stars 50 forks source link

BUG: registry narrative documentation fails with DALQueryError #568

Open bsipocz opened 1 week ago

bsipocz commented 1 week ago
docs/registry/index.rst F                                                                                                                                                                                        [100%]

======================================================================================================= FAILURES =======================================================================================================
_________________________________________________________________________________________________ [doctest] index.rst __________________________________________________________________________________________________
151       ivo://cds.vizier/j/aj/151/146 ...
152       ivo://cds.vizier/j/apj/727/14 ...
153   ...
154 
155 And to look for tap resources *in* a specific cone, you would do
156 
157 .. doctest-remote-data::
158 
159   >>> from astropy.coordinates import SkyCoord
160   >>> registry.search(registry.Freetext("Wolf-Rayet"),
UNEXPECTED EXCEPTION: DALQueryError: Field query: Could not parse your query: Expected ")", found ','  (at char 1046), (line:15, col:36)
Traceback (most recent call last):
  File "/Users/bsipocz/.pyenv/versions/3.12.1/lib/python3.12/doctest.py", line 1359, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest index.rst[9]>", line 1, in <module>
  File "/Users/bsipocz/munka/devel/pyvo/pyvo/registry/regtap.py", line 250, in search
    return query.execute()
           ^^^^^^^^^^^^^^^
  File "/Users/bsipocz/munka/devel/pyvo/pyvo/registry/regtap.py", line 268, in execute
    return RegistryResults(self.execute_votable(), url=self.queryurl)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/bsipocz/munka/devel/pyvo/pyvo/dal/query.py", line 336, in __init__
    raise DALQueryError(self._status[1], self._status[0], url)
pyvo.dal.exceptions.DALQueryError: Field query: Could not parse your query: Expected ")", found ','  (at char 1046), (line:15, col:36)
/Users/bsipocz/munka/devel/pyvo/docs/registry/index.rst:160: UnexpectedException
=============================================================================================== short test summary info ================================================================================================
FAILED docs/registry/index.rst::index.rst
================================================================================================== 1 failed in 34.02s ==================================================================================================

Does anyone else see the same problem?

ManonMarchand commented 1 week ago

Might have been introduced with #562 , I forgot to check the narrative docs before merging

bsipocz commented 1 week ago

No, that wasn't it, the problem exists on main, prior merging that PR. And the CI for that PR runs for the narrative docs, but it also runs all the other tests before the docs thus the problem was masked.

msdemlei commented 5 days ago

On Fri, Jun 28, 2024 at 10:04:26AM -0700, Brigitta Sipőcz wrote:

raise DALQueryError(self._status[1], self._status[0], url)

pyvo.dal.exceptions.DALQueryError: Field query: Could not parse your query: Expected ")", found ',' (at char 1046), (line:15, col:36) /Users/bsipocz/munka/devel/pyvo/docs/registry/index.rst:160: UnexpectedException Does anyone else see the same problem?

Not me, or at least not now. Dang.

This is a message from the RegTAP server, and reading the message would suggest that pyVO's query generator indeed produces bad ADQL code. By now, there are several code paths that depend on the remote server (for starters; and curse optional features!), so it's at least not very surprising that depending on some global state (for a different error message I would have said: "which registry you use") pyVO bugginess might vary.

I'd now be curious what that code is, and the fastest way to find that out probably is for someone with a failing test set to cause the error and then call me so I pick the failing query from the logs (I do have some, but I am clearing them out pretty aggressively, so I cannot see the server's logs from when your test failed any more). To find my phone number, try:

pyvo.registry.search(ivoid='ivo://org.gavo.dc')[0].get_contact()

ManonMarchand commented 5 days ago

Or use tcpdump locally so that you can read what is sent? I usually go with (close any annoying app that communicates too much via http):

prepare request

tcpdump -i any -w /tmp/http.pcap

send my request

killall tcpdump

read the file (with tcpdump -r) to find what exactly did I ask in the request

msdemlei commented 5 days ago

On Mon, Jul 01, 2024 at 03:34:53AM -0700, Manon Marchand wrote:

Or use tcpdump locally so that you can read what is sent? I usually go with (close any annoying app that communicates too much via http):

prepare request

tcpdump -i any -w /tmp/http.pcap

I like ngrep a lot for this kind of thing, but of course the rise of https makes these techniques substantially more complicated at the very least (IMHO one of the stronger reasons for considering forced https requests harmful, http://blog.tfiu.de/foced-https-redirects-considered-harmful.html). In this particular case, using get_RegTAP_query() instead of registry.search() would also work nicely...

But here I suspect having me pull the failure out of the server log really is the most straightforward way to proceed; who knows: It might even be some crazy stuff on the server side?