minvws / nl-kat-coordination

Repo nl-kat-coordination for minvws
European Union Public License 1.2
126 stars 57 forks source link

Replace `requests` with `httpx` #2299

Closed ammar92 closed 7 months ago

ammar92 commented 9 months ago

Is your feature request related to a problem? Please describe. I propose to replace requests with httpx for the following reasons:

Describe the solution you'd like For the transition I suggest to do the following:

Additional context This came to my mind because of the extra boilerplate code to handle base URLs. Also in an earlier issue where we actually used httpx, it blocked a GET request including a body, which is a good thing because it means httpx practices (modern) standards more strictly.

dekkers commented 9 months ago

Httpx seems to be the better library nowadays and given that it's mostly compatible it should indeed not be a lot of effort to switch. We would have to test it carefully, because as far as I know we have lot of requests between services that are not covered by integration tests.

underdarknl commented 9 months ago

Agreed, when the dust around 1.14 / reports settles we should have a look at this. Preferably at the beginning of a sprint, right after a release so we can test the changes for two sprints.

stephanie0x00 commented 7 months ago

Checklist for QA:

What works:

Looks good! No error messages are shown and the logs look fine too. Ready for merge.

What doesn't work:

n/a

Bug or feature?:

n/a