astropy / astropy/astroquery

make timeout work consistently across services and not misuse the configuration system

Open
#544 2 comments 0 reactions 0 assignees View on GitHub
infrastructure
Dominant language
Python
Stars
791
Forks
451
Avg merge
1d 3h
Merged PRs (30d)
4

Description

Nearly all of the astroquery services use a `timeout` argument, typically as part of `send_request` or similar. However, it seems like a few different ways of specifying this are available. I note `gama` has a `timeout` argument on the class, and `lcogt` has the same but it's `TIMEOUT`. `Nrao` meanwhile, looks similar, but is subtly different because it uses the _module-level_ instance `Nrao.timeout` instead of `self.timeout`. Others (e.g. `sdss`) instead have a default but allow the user to specify the timeout as an argument to a query function.

Moreover, in the cases I've looked at that use the configuration system, there's what appears to be a mis-use of the configuration system happening. (as an example, checkout out `sdss/core.py`). The `TIMEOUT` attibute is specified at the _class_ level, so the configuration item only gets touched at _import time_. If a user changes the configuration item as the astropy docs recommends, the code won't notice, because `conf.timeout` is never again accessed. Worse yet, the `TIMEOUT` attribute is only used to specify the _default_, so changing that _also_ does nothing.

This is very confusing from a user perspective - these should all be done uniformly. Personally I advocate for the "use a default from `conf.timeout`, unless specified by the user". So the queries woult all have `timeout=None` as an argument and usage would look like:

```
if timeout is None:
timeout = conf.timeout

commons.send_request(...,timeout=timeout,...)
```

@keflavich - do you have a sense if some of this is intentional? I (or someone else) could pretty easily go through and uniformly implement what I'm describing above, but maybe I missed some discussion of a reason for why all these different techniques are in use?

Contributor guide

Open the contributing guide

Research direction

Start by inspecting sdss/core.py and compare its timeout handling with the patterns in gama, lcogt, Nrao, and other service query functions. Determine which services use class, module-level, or query arguments, then review the issue's proposed default behavior; done means timeout handling is consistent and responds to configuration changes across the affected services.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.