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-37079: Add sites to RubinTV app #130

Merged
merged 91 commits into from
Jan 17, 2023
Merged

DM-37079: Add sites to RubinTV app #130

merged 91 commits into from
Jan 17, 2023

Conversation

ugyballoons
Copy link
Collaborator

Add multiple locations to the app, each with it's own GCS Bucket.
Add functionality to display Night Report data (temporarily disabled).
Fix bogus heartbeat status displays.

ugyballoons and others added 30 commits January 16, 2023 19:16
This commit also moves the models into a YAML file, installs dacine
to convert the resulting dict to Camera objects and changes the root
endpoint to yield a homepage that lists locations instead of the
summit's cameras
Fixes missing location var not passed to allsky templates
and deprecated download_as_string() replaced by download_as_bytes()
Also reorganise/rename js scripts with an eye to keeping
things simple for more cameras to be brought online
Also fixes several js bugs that prevented tables from showing/updating
Remove unnecessary date-based update test
Change date on allsky tests to match minimal Historical Data
Make tests failing on minimal load use full app
Fix the table control shifting width bug
Remove empty metadata check from refresher function
Rename and update incorrect docstring in historical data method
Replace Bucket.get_blob() with less expensive Bucket.blob()
Also fix historical night reports parameter type bug
Redraw table completely on checkbox change to allow keeping
of default heading order
Also attempt fixing misleading channel statuses on clicking
'back' in browser
src/rubintv/app.py Outdated Show resolved Hide resolved
src/rubintv/handlers/external/endpoints.py Outdated Show resolved Hide resolved
src/rubintv/models/historicaldata.py Outdated Show resolved Hide resolved
src/rubintv/models/historicaldata.py Outdated Show resolved Hide resolved
src/rubintv/models/historicaldata.py Outdated Show resolved Hide resolved
src/rubintv/models/models_assignment.py Show resolved Hide resolved
src/rubintv/models/models_assignment.py Show resolved Hide resolved
@@ -0,0 +1,37 @@
from pathlib import Path
Copy link
Collaborator

Choose a reason for hiding this comment

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

This whole file is a pattern I am not used to seeing, which worries me. You have no defined functions or classed, so it's more like a script, and you're importing cameras and locations which are defined in here. I'm not saying it's necessarily bad, but it's not something I'm familiar with, and it does make me wonder if there are good reasons to avoid this pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm... it did feel a little maverick as I was writing it. I'll look up what alternative patterns there are for loading yaml into dataclasses

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, if these were in get_x() functions I wouldn't have blinked, it's just the script-like nature of the file and importing those variable felt a little unusual.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We agreed OOB to fix this on the next ticket in favour of deploying the new version for tonight's run.

@mfisherlevine mfisherlevine merged commit ee5e5f6 into master Jan 17, 2023
@mfisherlevine mfisherlevine deleted the tickets/DM-37079 branch January 17, 2023 19:57
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