kolibri
kolibri copied to clipboard
Replace existing usages of `requests` with `NetworkClient`
Background
We've made our NetworkClient
utility more flexible and incorporated sensible default timeouts and a custom user agent. It's also now smarter when given a NetworkLocation
.
Goals
There are several usages of requests
that bypass the custom behavior of NetworkClient
. Ideally they should be refactored to use the NetworkClient
instead. Some additional functionality and flexibility may be required to handle all other use cases of requests
.
Hi @rtibbles @bjester, can I give it a try with this task? Could you please provide more info?
Yes, that would be great.
The first thing to do would be to look for places in the codebase where the requests
module is used directly to make requests. Making a TODO list of them in a comment here would be a good first step, and we can weigh in, in case any are not needed to be migrated.
The next step would then to start replacing them with usage of the NetworkClient class. The best example of using the NetworkClient class for requests can be found here: https://github.com/learningequality/kolibri/blob/e67339ed5db50c37dea02a1f355a91ad249c2415/kolibri/core/discovery/api.py#L91
The requests
module should be replaced in the following Kolibri modules:
- [ ] core
- [ ] core/analytics
- [ ] core/auth
- [ ] core/auth/management
- [ ] core/auth/management/commands
- [ ] core/auth/utils
- [ ] core/content
- [ ] core/content/utils
- [ ] core/utils
- [ ] plugins/setup_wizard
- [ ] plugins/user_profile
The requests module will still be used in utils
(Kolibri modules shouldn't be imported there) and in the build_tools
. Is that right @rtibbles?
Should the exceptions be replaced as well? Or just the HTTP requests?
The requests module should be replaced in the following Kolibri modules
@andreamisuraca I'm sorry, we missed this. We'll reach out.
Hi @andreamisuraca - apologies for missing this! We definitely don't need to replace in the build_tools.
One of the main motivations for using the NetworkClient is to ensure we have a user agent that shows this is Kolibri - we may want to look into how we can use it in utils
as well - but we can do that as a follow up issue, as it may require some additional work to extricate from other code.
Exceptions can be kept the same, as it is still using requests under the hood - we're just wrapping it to ensure consistency!
May I work on this?
Hi @thesujai, yes, thank you