redkyn / assigner

:black_nib: Automatically assign programming homework to students on GitLab.
MIT License
27 stars 6 forks source link

CanvasAPI _decode_links is too clever #47

Closed LinuxMercedes closed 7 years ago

LinuxMercedes commented 7 years ago

I have no idea what that function does! @islamelnabarawy, is it possible to implement that with the urlparse library, or at least add some comments explaining what's up?

islamelnabarawy commented 7 years ago

Well, it's kinda hard to explain cuz it's a bit hack-ish. The response from Canvas typically has something like the following in its Link field of the HTTP response header:

<https://mst.instructure.com/api/v1/courses?page=1&per_page=10>; rel="current",
<https://mst.instructure.com/api/v1/courses?page=1&per_page=10>; rel="first",
<https://mst.instructure.com/api/v1/courses?page=1&per_page=10>; rel="last"

It uses that for paginating the query results, which the _get_all_pages function handles. The _decode_links function basically extracts the links and titles from that string, and returns a dictionary that would look like this:

{
  'current': '/api/v1/courses?page=1&per_page=10', 
  'last': '/api/v1/courses?page=1&per_page=10', 
  'first': '/api/v1/courses?page=1&per_page=10'
}

The _get_all_pages function is also hack-ish, mostly because I wrote this code as a proof-of-concept to see if I could extract data from Canvas API, and then it ended up making it into production as usual.

I will create a pull-request with documentation and improvements to this function's code as soon as I have the chance. I've got two other code projects and two papers that I'm working on this week, though, so no promises on how soon that will happen. EDIT: strike that, I'll create it and post it now.

LinuxMercedes commented 7 years ago

Thanks! That is an oddball format...

(For future reference, don't feel like you /have/ to fix stuff right away; I know you've got other stuff that's more important than maintaining a tool you're not even using this semester!)