-
Notifications
You must be signed in to change notification settings - Fork 12
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
[Exploration] Office Hours Querying Enhancements #631
base: main
Are you sure you want to change the base?
Conversation
( @wzahrt // @atoneyd // @ItIsAndrewL), no todos or anything pressing, but pinging you all on this pull request to alert you of potential (needed) optimizations coming down the pipe to load office hours data for the TV signage, where applicable (specifically if any APIs are querying with joins between course sites, office hour events, and tickets). |
@KrisJordan, removing the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM!
This PR explores refactoring the permission check functions used in the common office hours CRUD backend service functions. The idea of these refactors is to load as little information as possible while churning over many courses, members, and sections. Some of these ideas are based off of SQLAlchemy documentation, examples, research, and some suggested optimizations from AI.
@KrisJordan and @jadekeegan, would like to hear your thoughts on this. @KrisJordan, I think to solve the problem noticed in larger courses like COMP 301, it might be helpful if you can gather any other insights on
502
errors occurring on the backend side, as well as the number of observations in the tables we are dealing with (particularlyticket
,office-hours
(formerly office hour events), andsection_member
).