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

HP-2082 | Update Graphene, Django and Python #467

Merged
merged 16 commits into from
Feb 6, 2024

Conversation

charn
Copy link
Contributor

@charn charn commented Jan 30, 2024

Bumped most of requirements in the project. Notable mentions:

  • Use latest Django LTS 4.2
  • graphene and graphene-django bumped to latest
  • Bump psycopg 3 as recommended in Django 4.2 documentation
  • Use a maintained version django-enumfields2
  • Lift graphene version limit for graphene-validator

Includes a lot fixes due to deprecated features and functionality in the updated packages.

TODO Fix data loaders to work with the new packages.

@charn charn requested review from AntiAki-M and a team January 30, 2024 15:36
@charn charn force-pushed the HP-2082-python-graphene-django-update branch 3 times, most recently from b093431 to b60a6f0 Compare January 30, 2024 19:23
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024

Codecov Report

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

Comparison is base (61fabad) 90.92% compared to head (b60a6f0) 95.50%.

❗ Current head b60a6f0 differs from pull request most recent head 077b6de. Consider uploading reports for the commit 077b6de to get more accurate results

Files Patch % Lines
open_city_profile/decorators.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #467      +/-   ##
==========================================
+ Coverage   90.92%   95.50%   +4.57%     
==========================================
  Files          55      207     +152     
  Lines        2821     8186    +5365     
  Branches      334      982     +648     
==========================================
+ Hits         2565     7818    +5253     
- Misses        200      282      +82     
- Partials       56       86      +30     

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

charn added 3 commits January 30, 2024 21:46
Current production environments use a more recent version
of postgresql and postgis. This update will also be required
for raising the version of Django.

Refs: HP-2082
Use python docker image. Copy useful tools from the helsinkitest
image.

Refs: HP-2082
@charn charn force-pushed the HP-2082-python-graphene-django-update branch 2 times, most recently from 39c37c1 to 5f13609 Compare January 30, 2024 19:57
Copy link
Contributor

@AntiAki-M AntiAki-M left a comment

Choose a reason for hiding this comment

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

Reviewed changes and tested on my my machine and the tests pass, and the server runs without issues.

charn added 13 commits February 2, 2024 14:42
Bumped most of requirements in the project. Notable
mentions:
- Use latest Django LTS 4.2
- graphene and graphene-django bumped to latest
- Bump psycopg 3 as recommended in Django 4.2 documentation
- Use a maintained version django-enumfields2
- Lift graphene version limit for graphene-validator

Also:
- Add ipython for better python console

Refs: HP-2082
Also fix isort command for bumped isort.

Refs: HP-2082
Language is a enum instead of a string.

Refs: HP-2082
…e_profile

Test was using methods which were apparently deprecated.

Refs: HP-2082
Django DjangoFilterConnectionField changes order_by filter
arguments to snake case, so field mapping should have
arguments as snake case.

Refs: HP-2082, graphql-python/graphene-django@c049ab7
When there's an exception/error happening in the API,
the data part in the response is None, instead of the
data part containing some keys.

Refs: HP-2082
Schema generation is more verbose, update the
test content to reflect that.

Refs: HP-2082
Updated packages add tiny changes to the generate the schema. Mostly just
ordered differently, but also more modern federation related types.

Refs: HP-2082
Same information seems to available under a different
attribute.

Refs: HP-2082
Data loaders that exist are not fully compatible with new versions of
graphene and graphene-django. DjangoConnectionField doesn't seem to handle
loaders correctly and instead return errors like:

"Cannot return null for non-nullable field EmailNodeConnection.edges."

So for now, data loaders will be disabled for this field type.

Use graphql-sync-dataloaders to make other types of fields work with
data loaders.

Some GitHub issues for reference:
- graphql-python/graphene-django#1394
- graphql-python/graphene-django#1263
- graphql-python/graphene-django#1425

Refs: HP-2082
@charn charn force-pushed the HP-2082-python-graphene-django-update branch from 5f13609 to 077b6de Compare February 2, 2024 13:40
@charn charn marked this pull request as ready for review February 2, 2024 13:40
Copy link

sonarqubecloud bot commented Feb 2, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

16.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@charn charn merged commit 878e322 into main Feb 6, 2024
5 of 6 checks passed
@charn charn deleted the HP-2082-python-graphene-django-update branch February 6, 2024 09:00
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.

3 participants