make timeout work consistently across services and not misuse the configuration system
- 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
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