search

tl-its-umich-edu / tool-migration

App for migrating external (LTI) tools in Instructure's Canvas LMS.
1 stars 3 forks source link

term lookup causing error #40

Open lsloan opened 10 months ago

lsloan commented 10 months ago

Term lookup API call is not reading all of the pages from the API. It reads only the first page and stops. If the later usage of the data from the call doesn't include the term number from the app parameter, the lookup fails.

lsloan commented 10 months ago

This was temporarily disabled in #41.

It needs to be corrected, preferably by resolving #43 and reenabling this feature.

ssciolla commented 9 months ago

Hi @lsloan, this is certainly a bug and something I overlooked before. However, I'd caution you to not jump to #43 too quickly, since I'm not sure if canvasapi supports async/await.

It would be easy enough to modify AccountManager.get_term_names to use API.get_results_from_pages instead of API.get. This is already being done when fetching courses here: https://github.com/tl-its-umich-edu/tool-migration/blob/d1d2716fa5ea45b23d8581d1146fab7fd57f52f1/migration/manager.py#L73-L78

Wishing you the best in 2024.