Skip to content

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented Mar 7, 2024

This PR modifies the calib validity hack introduced on #129 to optionally purge the calib cache, preventing conflicts if two calibs are loaded on the same day. While calibs do not change mid-day in production, our integration tests (especially HSC RC2) mix a wide range of "real" observing dates in a single run, and the previous hack would break them.

For some reason, duplicate calibs are getting preloaded. This should
help find them.
@kfindeisen kfindeisen marked this pull request as ready for review March 8, 2024 19:15
@kfindeisen kfindeisen requested a review from hsinfang March 11, 2024 17:48
The option prevents crashes when calibs are fed out of chronological
order to our validity workaround code. It should not be set for any
production run.
The S3 code always tries to flush even though flushing is not supported
for small files; the file gets written correctly, but the user can't
stop the attempt.

# VALIDITY-HACK: local-only calib collections break if calibs are not requested
# in chronological order. Turn off for large development runs.
cache_calibs = bool(int(os.environ.get("DEBUG_CACHE_CALIBS", '1')))
Copy link
Collaborator

Choose a reason for hiding this comment

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

All other parameters we load from environment are in activator.py. Would it be easier to manage if this goes there too?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's... not actually true. There are quite a few calls to os.environ here, I was just lazy and buried the others in the code. 😬

If the variables got moved to activator.py, how would we make them accessible to this module's code? There's too many to be good parameters to the MiddlewareInterface constructor, and in any case a lot of them (including cache_calibs) are Middleware implementation details that activator shouldn't know about.

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.

2 participants