FC-0001: replace EdxRestAPIClient with OAuthAPIClient

[dyudyunov]

closes openedx/public-engineering#48

All the changes could be separated into two types:

1. Replacing EdxRestAPIClient with OAuthAPIClient

• new client primarily used from the core/models.py SiteConfiguration cached property. There are base API URLs stored there too instead of the old clients.
• there are few places where pure requests are used (primary in the management commands) because there is no need for Session and authentication there.
• responses collected from the new client requests require additional .raise_for_status and .json() to be called (this was working under the hood in the old Slumber-based client)
• replaced Slumber error types with the appropriate requests ones. I've used following mapping:
  • SlumberBaseException -> RequestException
  • SlumberHttpBaseException / HttpNotFoundError / HttpClientError / HttpServerError -> HTTPError

2. Tests reworked

• The httpretty library was replaced with responses because it had issues with keeping connections with requests.Session.
• although responses usage is pretty the same as httprettys, it requires a more strict URL format (including the query parameters). It also can't be activated at the class level using the decorator

[openedx-webhooks]

Thanks for the pull request, @dyudyunov! I've created OSPR-6589 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[natabene]

@dyudyunov Thank you for your contribution. It is on our radar now, we will review it in the coming weeks.

[colinbrash]

@dyudyunov given the large number of changes, would you please add as many details as you can to the description? Summarize the different changes that were made throughout all the files, describe common types of changes, identify any areas of particular risk, etc.

Also, if it is possible to split commits for different types of changes, that would help immensely. For example, if we could review the library change separately from the EdxRestApiClient change.

Thanks!

[feanil]

Changes generally make sense to me, really nice cleanup. My only constructive feedback wolud be that I would really love to see something this big broken up into smaller PRs in the future. Not giving an approval because I'd like the team that maintains this code to go through it but I think this is good to go.

[dyudyunov]

@dyudyunov given the large number of changes, would you please add as many details as you can to the description? Summarize the different changes that were made throughout all the files, describe common types of changes, identify any areas of particular risk, etc.

Also, if it is possible to split commits for different types of changes, that would help immensely. For example, if we could review the library change separately from the EdxRestApiClient change.

Thanks!

Hello @colinbrash
Thank you for your suggestions!
I've updated the description for the PR. I also could split the commit into two (code part / test part which could be also called "client replacement" / "tests updates") if it will be more convenient to review it that way.

Changes generally make sense to me, really nice cleanup. My only constructive feedback would be that I would really love to see something this big broken up into smaller PRs in the future. Not giving an approval because I'd like the team that maintains this code to go through it but I think this is good to go.

Hi, @feanil !
I totally agree with you :)
I didn't expect that quantity of changes before I've started to work with tests :)
But yeah, I've got your point, will split such work into smaller parts in the future.
Thanks

[dyudyunov]

Hi all! I've split this into two commits, hope this will ease the review

@feanil @colinbrash FYI

[dianakhuang]

This looks good to me overall! There are some logging changes, but they're minor enough that I don't think they're worth worrying about. I'm going to wait for the Revenue Squad to also get a chance to review before we merge, though.

[dyudyunov]

I've pushed commit with 1 change (found the Slumber exception usage I've missed before)
@dianakhuang @feanil @colinbrash please, take a look

[dyudyunov]

rebased the branch on the top of master branch

[dyudyunov]

there were no updates for translations files from my side, I don’t know why the pipeline failed to check them

[feanil]

It looks like the translations issue is unrelated and someone is working on fixing it: #3702

We'll probably just need to rebase once it's fixed.

[openedx-webhooks]

@dyudyunov 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.