kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Replace existing usages of `requests` with `NetworkClient`

Open bjester opened this issue 1 year ago • 7 comments

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.

bjester avatar Jul 26 '23 14:07 bjester

Hi @rtibbles @bjester, can I give it a try with this task? Could you please provide more info?

andreamisuraca avatar Oct 10 '23 13:10 andreamisuraca

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

rtibbles avatar Oct 10 '23 17:10 rtibbles

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?

andreamisuraca avatar Oct 11 '23 08:10 andreamisuraca

The requests module should be replaced in the following Kolibri modules

@andreamisuraca I'm sorry, we missed this. We'll reach out.

MisRob avatar Dec 15 '23 06:12 MisRob

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!

rtibbles avatar Dec 15 '23 15:12 rtibbles

May I work on this?

thesujai avatar Apr 18 '24 03:04 thesujai

Hi @thesujai, yes, thank you

MisRob avatar Apr 19 '24 06:04 MisRob