astropy / astroquery

Functions and classes to access online data resources. Maintainers: @keflavich and @bsipocz and @ceb8
http://astroquery.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
702 stars 396 forks source link

Tap/TAP+ unsuccessful in async mode to some valid TAP services, relies on potentially optional features #1160

Closed theresadower closed 4 months ago

theresadower commented 6 years ago

When using the astroquery.utils.tap library against several valid TAP services other than GAIA, synchronous mode queries work but asynchronous ones hang and eventually time out with exceptions. Working on this issue for Jupyter notebooks for NAVO demos, we found the issue to be caused by how the library submits asynchronous calls using a parameter read as optional in the basic UWS standard (http://www.ivoa.net/documents/UWS/20161024/index.html , UWS 1.1, section 2.2.3.1. Creating a Job), but not described in UWS usage in the TAP standard, which can be read as to supersede it (http://www.ivoa.net/documents/TAP/20100327/REC-TAP-1.0.pdf, TAP 1.0, section 5, use of UWS, separate Creating a Query and Running a Query sections.)

The TapPlus call is (as a POST, but shown this way for clarity) http://host/tap/async?request=doQuery&query=xxx&lang=ADQL&phase=run

The optional parameter phase=run essentially quick-starts the job, rather than sending two separate calls as per: http://host/tap/async?request=doQuery&query=xxx&lang=ADQL then http://host/tap/async/jobid/phase?phase=run

At MAST and HEASARC, we have made server-side changes to our TAP servers to accept the PHASE=RUN argument on asynchronous job creation, but there remains some internal argument as to whether it is correct or required, especially as several implementors have independently not initially supported it. Changing TapPlus to separately create and run jobs, another valid pattern, may keep the issue from happening with other new TAP services.

keflavich commented 6 years ago

cc @jcsegovia - can you address this issue?

jcsegovia commented 6 years ago

@theresadower thanks for spotting this. Yes, TAP library launches jobs adding phase=run parameter. I understand that there TAP implementations that cannot handle this. I will add methods to create jobs and run jobs in order to make this library more compatible.

brianmajor commented 6 years ago

Hi @jcsegovia & @theresadower We at the CADC have also encountered this issue in TapPlus and would like to volunteer to make the necessary changes. Is that alright? --Brian

jcsegovia commented 6 years ago

Hi @brianmajor, @theresadower , currently, we are working in a new version of TAP+ (we are adding share capabilities, table edition and private user uploads: these capabilities are available by command line, but they have not been implemented in Astroquery module). Also, we were planning to fix this problem with phase=run parameter. We expect to release these changes by mid next month. @brianmajor thanks for volunteering to make these changes, but, if you do not mind, as we are modifying some core functions, I would prefer we do these changes (in order to avoid conflicts). Thanks again.

brianmajor commented 6 years ago

Hi @jcsegovia

We have already created a pull request for the phase=run param problem, here: https://github.com/astropy/astroquery/pull/1240

Would you consider accepting this work? I imagine it is a similar solution to what you'd come up with. We are looking to get a working TAP client into the hands of our users sooner than later.

The new features you are adding sound exciting, looking forward to seeing that. Please let us know if there's anything on which we can help contribute. Cheers.

cbanek commented 5 years ago

Hi! I just want to chime in and say I've been seeing the same behavior against the TAP service that I use. Sync works fine, but async does not. It seems to be about the same PHASE issue as mentioned above. What is the current way to have this work, or what is the timeline to have this resolved? I see there are a few pull requests in flight.

bsipocz commented 4 months ago

I'm closing this issue as we have the setup for a while that TapPlus is to be used by the ESA modules, and end users and other services are directed towards using pyvo's TAP. So any issues or missing features should be reported there.