Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-41880: Support ChainedDatastore for Butler server #955

Merged
merged 5 commits into from Feb 2, 2024

Conversation

dhirving
Copy link
Contributor

@dhirving dhirving commented Feb 2, 2024

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

Implement prepare_get_for_external_client in ChainedDatastore so that Butler server will work with ChainedDatastore.  The Butler deployment on idfprod uses ChainedDatastore.
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (97c358c) 88.40% compared to head (3a6d763) 88.41%.

Files Patch % Lines
python/lsst/daf/butler/datastores/fileDatastore.py 70.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #955   +/-   ##
=======================================
  Coverage   88.40%   88.41%           
=======================================
  Files         303      303           
  Lines       39145    39163   +18     
  Branches     8252     8256    +4     
=======================================
+ Hits        34606    34625   +19     
  Misses       3332     3332           
+ Partials     1207     1206    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

When the DatasetRef contains datastore records, we now use them as the definitive source of truth for FileDatastore.knows().  This allows the function to avoid database I/O when datastore records are attached.

The type of DatasetDatastoreRecords was made more exact -- code is assuming the truthy/falsy behavior of the Iterable as if it is was a list, and in practice it is always a list.
For Butler server, DatasetRef passed to datastore will always have datastore records attached.  Avoid duplicate database queries for ChainedDatastore by using these records to select which datastore to use.
@dhirving dhirving marked this pull request as ready for review February 2, 2024 19:01
Copy link
Member

@timj timj 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.

python/lsst/daf/butler/datastores/chainedDatastore.py Outdated Show resolved Hide resolved
Co-authored-by: Tim Jenness <tjenness@lsst.org>
@dhirving dhirving merged commit af21ca5 into main Feb 2, 2024
18 checks passed
@dhirving dhirving deleted the tickets/DM-41880 branch February 2, 2024 23:10
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.

None yet

2 participants