Skip to content

Conversation

@kfindeisen
Copy link
Member

This PR adds some special-case handling for nextVisit messages with empty filters (as is common for engineering data). The cases are handled at a fairly low level (_export_calibs and _export_skymap_and_templates) because I don't see an obvious way to unify what's essentially a query tweak. This may need to be revisited on #204, which reworks how the preload queries are executed.

Since ButlerCollections.defaults is only specced as a sequence, and all
three contexts require a specific class (list or set), the converting
constructor calls can't be removed.
The SAL spec allows filters to be the empty string to indicate that the
script did not specify a filter. If such a message is received, do not
constrain dataset queries by filter.

For this option to be useful, the dataset cache must be large enough to
store one calib per filter. The unit tests' default cache has been
expanded accordingly.
Visits with unspecified filter may download calibs or templates for all
filters. Such a download must not overflow the local cache by itself.
However, since image-type calibs and coadds are among the largest input
datasets, we must be careful not to expand the cache more than
necessary.
@kfindeisen kfindeisen requested a review from hsinfang November 5, 2024 18:15
Copy link
Collaborator

@hsinfang hsinfang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the idea and for the fix.
I'm worried about the growing list of dataset type names that we have to include in this code. I see there is a TODO there; maybe we want to think about it sooner.

@kfindeisen kfindeisen merged commit da2c4c5 into main Nov 5, 2024
5 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-47387 branch November 5, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants