Add new course search reset endpoint and extend SIS API documentation #41
Conversation
Renamed and split the SIS search state reset logic into two functions: init_class_search for term initialization and reset_class_search for subject reset. Updated usage in sis_scraper.py to use the new init_class_search function for initializing the search state before fetching class data.
There was a problem hiding this comment.
Pull request overview
This pull request adds a new reset_class_search() endpoint for resetting course search state and extends SIS API documentation with comprehensive parameter and return type annotations. The PR also renames the existing course search initialization function from reset_class_search to init_class_search to better reflect its purpose.
- Added
@paramand@returndocumentation tags to all SIS API functions - Introduced a new
reset_class_search()function for subsequent subject searches (not currently utilized) - Renamed the existing search initialization function to
init_class_search()with updated documentation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| sis_scraper/sis_scraper.py | Updates import and function call from reset_class_search to init_class_search, with corresponding comment update |
| sis_scraper/sis_api.py | Adds comprehensive parameter and return type documentation across all API functions; renames reset_class_search to init_class_search; introduces new reset_class_search() function with different signature and behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jzgom067
left a comment
jzgom067
left a comment
There was a problem hiding this comment.
Jack, I know you're not gonna like this pull request since it adds two, very different things.
Yes you're right, this pull request is terrible. I hope the extra-long name you had to write for it serves as a permanent reminder of your mistakes.
What?
Jack, I know you're not gonna like this pull request since it adds two, very different things.
This pull request adds a new function for resetting the course search state on the SIS server that is a faster alternative to the existing one. In addition, I've added parameter and return type documentation for all SIS API functions.
Closes #27.
Why?
I thought this new course search reset endpoint would be a simple drop-in replacement for the function the scraper currently uses. However, upon inspecting the scraper code, a client session is spawned for every subject, for every term. Each of these sessions only needs to reset the course search state once, so there's actually no use for this new course search reset endpoint.
BUT, it could be useful depending on whether we decide to refactor the scraper logic in the future. Although I'm not sure why we would downgrade to having each session iterate synchronously through multiple subjects in a term.
I added more documentation to the code because why not.
How?
I wrote a new function and didn't use it.
Testing?
The scraper code didn't change except for one function name change.
Anything Else?
Have a good day.