Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⬆️ [#2040] Upgrade python to 3.11 and zgw-consumers to 0.28.0 #977

Merged
merged 9 commits into from
Jan 29, 2024

Conversation

stevenbal
Copy link
Contributor

@stevenbal stevenbal commented Jan 22, 2024

issue: https://taiga.maykinmedia.nl/project/open-inwoner/issue/2040

This PR upgrades to python3.11 and zgw-consumers to 0.28.0, in this new version of zgw-consumers we no longer do schema parsing to determine request paths, which removes latency that was caused by that

Notes:

  • we should probably go over every integration manually on test/staging to see if it still works. Since a lot of stuff is mocked in tests, we won't detect problems like missing CRS headers for Zaken API until we test it on an actual environment. Also because we no longer rely on operationIds to determine request paths, we'll have to see if eSuite actually implements the same request paths as Open Zaak
  • get_paginated_results doesn't work with the new client, created another issue for this -> https://taiga.maykinmedia.nl/project/open-inwoner/task/2060
  • also created an issue to refactor the client usage to be more in line with Open Forms, that way we could eventually move common code to a package

@stevenbal stevenbal marked this pull request as draft January 22, 2024 12:18
@stevenbal stevenbal force-pushed the feature/2040-upgrade-zgw-consumers branch 6 times, most recently from 568b8fd to 65a18c1 Compare January 23, 2024 14:56
@stevenbal stevenbal changed the title Feature/2040 upgrade zgw consumers ⬆️ [#2040] Upgrade python to 3.11 and zgw-consumers to 0.28.0 Jan 23, 2024
@stevenbal stevenbal force-pushed the feature/2040-upgrade-zgw-consumers branch 2 times, most recently from c7df082 to 4bad33b Compare January 23, 2024 15:29
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (b91b39e) 94.77% compared to head (cedcd4c) 94.74%.

Files Patch % Lines
src/open_inwoner/utils/api.py 75.51% 12 Missing ⚠️
src/open_inwoner/openzaak/catalog.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #977      +/-   ##
===========================================
- Coverage    94.77%   94.74%   -0.03%     
===========================================
  Files          861      862       +1     
  Lines        30310    30265      -45     
===========================================
- Hits         28726    28676      -50     
- Misses        1584     1589       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stevenbal stevenbal force-pushed the feature/2040-upgrade-zgw-consumers branch 2 times, most recently from 16d2a3c to d2c092b Compare January 25, 2024 11:57
@alextreme
Copy link
Member

Depends on upgrading cindy+sandra to Debian 12

src/open_inwoner/conf/base.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Bartvaderkin Bartvaderkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At last a newer python!

Removing the operation ID's feels brave, but if ZGW-consumers believes in it then lets go for it.

I put two notes but not blocking.

@@ -1 +1 @@
3.9
3.11
Copy link
Contributor

@Bartvaderkin Bartvaderkin Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github's 2-way changes diff warns there is no newline here now, not sure if that is something.

Comment on lines +60 to +84
class JSONParserClient(APIClient):
"""
Simple layer on top of `ape_pie.APIClient` to attempt to convert the response to
JSON and check that the request is successful (and raise the correct exceptions if not)
"""

def request(
self,
*args,
**kwargs,
) -> Union[List[Object], Object]:
response = super().request(*args, **kwargs)
try:
response_json = response.json()
except Exception:
response_json = None

try:
response.raise_for_status()
except requests.HTTPError as exc:
if response.status_code >= 500:
raise
raise ClientError(response_json) from exc

return response_json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels weird this is not part of either apie_pie or ZGW-consumers but heh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I think we'll want this for more projects

@alextreme
Copy link
Member

Mag gemerged worden zodra de conflicts zijn geresolved

@stevenbal stevenbal force-pushed the feature/2040-upgrade-zgw-consumers branch from 5073d70 to cedcd4c Compare January 29, 2024 10:37
@stevenbal stevenbal marked this pull request as ready for review January 29, 2024 10:38
@stevenbal stevenbal merged commit df292b0 into develop Jan 29, 2024
16 checks passed
@stevenbal stevenbal deleted the feature/2040-upgrade-zgw-consumers branch January 29, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants