We would like advice on the best way to get a butler to @KSK’s new indexing code.
@KSK implemented his code as task LoadIndexedReferenceObjectTask. This uses a butler and a config parameter for the name of a dataset type to set up loading of index files. When a user requests all objects in a given region the task figures out which index files to read, reads them with the butler and returns the relevant data. At present the task performs this initial setup in its constructor (thus reporting certain kinds of errors as early as possible). However, this means its constructor needs a butler and tasks are not set up to do this. The question is how to solve this problem.
Choices we have considered:
-
Pass a butler to the loader task’s constructor.
This is a natural thing to do from the loader task’s perspective. The index file loader can be set up as the task is constructed, any errors reported, and the task is then ready to provide reference objects on demand.
However, it is a challenge because no other task is constructed with a butler at present, and we don’t see that changing. Only tasks that do I/O even need a butler, and they receive one (via a data ref) to their
run
method. Nonetheless, we could pass the butler to all task constructors and most would ignore it (except for constructing subtasks).If we go this route then we should consider whether tasks that perform I/O should receive and store the butler at construction time, rather than receiving it in the
run
method. I doubt this is wise because even command-line tasks only do I/O in a small subset of methods, and those are made explicit by passing in the butler. -
Pass a butler to the loader task’s
run
method and all other puplic methods that retrieve data.I feel it clutters up the loader task and calling tasks, including several methods of reference loaders. Note that the butler argument would be ignored after the first call, since the reference loading code caches it.
-
Make the butler load reference objects, given a region.
This is our preferred solution. It makes
CameraMapper
more complicated, but it basically means moving the new code into the camera mapper.In theory this also provides future expandability. Given a dataset name, the camera mapper can figure out which kind of reference object loader is required and instantiate the correct one. This is not necessarily difficult; a small text file can contain the required class.
-
Do not use the butler.
We dislike this solution because it breaks the abstraction of I/O and because we would need some other way than the butler to locate the reference data. (The current astrometr.net-based solution uses an environment variable, but we don’t consider that a good long-term solution).