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-42469: Update fan-out for the nextVisit schema change #7

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

hsinfang
Copy link
Collaborator

No description provided.

@hsinfang hsinfang force-pushed the tickets/DM-42469 branch 4 times, most recently from 351eb8d to 59cba21 Compare January 16, 2024 23:10
Comment on lines 288 to 297
latiss_gauge.inc()
fan_out_message_list = (
next_visit_message_updated.add_detectors(
"LATISS",
dataclasses.asdict(next_visit_message_updated),
latiss_active_detectors,
)
Copy link
Member

Choose a reason for hiding this comment

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

It seems a bit odd to have add_detectors be triggered by a match on salIndex when we're already assuming that next_visit_message_updated has a valid instrument. Is this to handle the weird edge case of upload.py HSC? If so, it might be a good idea to add a comment to the match block.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now that we have a valid instrument we probably can refactor this. Indeed the weird case of upload.py HSC incomplete detectors will make the refactoring uglier. I'll add a comment.

The field "instrument" was added to the schema of summit nextVist
messages. Therefore the fan-out service no longer needs to add the
"instrument" field to the fanned-out messages.
@hsinfang hsinfang merged commit 8fc1eab into main Jan 22, 2024
2 checks passed
@hsinfang hsinfang deleted the tickets/DM-42469 branch January 22, 2024 18:04
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