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

Feature/thumbnails #11

Merged
merged 23 commits into from
Jun 3, 2024
Merged

Feature/thumbnails #11

merged 23 commits into from
Jun 3, 2024

Conversation

mgdaily
Copy link
Contributor

@mgdaily mgdaily commented May 16, 2024

These are the changes needed to support the ingester and science archive in storing thumbnails. Here we just add some validation to make sure some additional thumbnail metadata is included in the payload. Also add a set of thumbnail filetypes which the ingester uses to know where to send the file.

@mgdaily mgdaily requested a review from jnation3406 May 16, 2024 23:40
@coveralls
Copy link

coveralls commented May 16, 2024

Pull Request Test Coverage Report for Build 9355323460

Details

  • 69 of 73 (94.52%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 96.391%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocs_archive/input/thumbnailfile.py 20 24 83.33%
Totals Coverage Status
Change from base Build 7703957207: 0.05%
Covered Lines: 1095
Relevant Lines: 1136

💛 - Coveralls

Matt Daily added 3 commits May 17, 2024 15:05
Remove mapping in settings, update filestore path, etc...
Also add a couple of tests for thumbnail files
Copy link
Member

@jnation3406 jnation3406 left a comment

Choose a reason for hiding this comment

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

Sorry just noticed this PR! I think we need to add a few fields to required headers, but otherwise looks good.

@@ -44,6 +44,9 @@ def get_tuple_from_environment(variable_name, default):
# Fits headers that must be present
REQUIRED_HEADERS = get_tuple_from_environment('REQUIRED_HEADERS', 'PROPID,DATE-OBS,INSTRUME,SITEID,TELID,OBSTYPE,BLKUID')

# Metadata that must be present when ingesting a thumbnail
REQUIRED_THUMBNAIL_METADATA = get_tuple_from_environment('REQUIRED_THUMBNAIL_METADATA', 'frame_basename,size')
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need the site_id, instrument_id, and observation_day to be required so we know where to store/retrieve the thumbnails in the daydirs. Actually looking above I think we need to require observation_day for LCO fits files too since its required for that path!

Matt Daily added 13 commits May 21, 2024 15:29
This is so that the archive can call this function in the thumbnail model without making an extra join to get all of the header information from the associated frame. Rather, it can provide a set of metadata to simply build the correct file path.
We keep the same interface for get_filestore_path so that the ingester can utilize it, but add a static method that takes a subset of frame metadata to also generate
a filestore path so that we don't have to pull the entire frame header from another table in the archive. Add thumbnail file and test for ingesting to filestore
Break out the INTEGER_TYPES from the fits file to the settings module
This is to address the 422 errors we ocassionally get from coveralls
@mgdaily mgdaily merged commit 77ff9ec into main Jun 3, 2024
10 of 11 checks passed
@mgdaily mgdaily deleted the feature/thumbnails branch June 3, 2024 18:24
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