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

Get rid of Deprecation Warnings #1530

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

patillacode
Copy link

@patillacode patillacode commented Nov 22, 2023

In my current project which depends on graphene (3.3) I encountered my logs unusable since they were heavily polluted with the following warning:

/home/wagtail/.local/lib/python3.9/site-packages/graphene/utils/subclass_with_meta.py:46: DeprecationWarning: Creating a DjangoObjectType without either the `fields` or the `exclude` option is deprecated. Add an explicit `fields = '__all__'` option on DjangoObjectType CollectionObjectType to use all fields

While trying to fix it just by adding the fields="__all__" option I stumbled upon this other one:

/home/wagtail/.local/lib/python3.9/site-packages/graphene/utils/subclass_with_meta.py:46: DeprecationWarning: Defining `exclude_fields` is deprecated in favour of `exclude`.

I believe this PR would make those warnings disappear without breaking anything AFAIK, at least that is what I found while testing it.

pass fields param

last try

last try 2

re-try
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

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

Comparison is base (5fb7b54) 96.01% compared to head (63ca079) 95.96%.

Files Patch % Lines
graphene/utils/subclass_with_meta.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1530      +/-   ##
==========================================
- Coverage   96.01%   95.96%   -0.05%     
==========================================
  Files          51       51              
  Lines        1755     1759       +4     
==========================================
+ Hits         1685     1688       +3     
- Misses         70       71       +1     

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

@ssyberg
Copy link

ssyberg commented Mar 13, 2024

Would it be possible to get this merged?

@erikwrede
Copy link
Member

Sorry for the late response. To me it looks like this is an issue that should be fixed in graphene_django, namely removing the use of a deprecated field in graphene. I'd prefer a solution that uses the newest standard in graphene instead of un-deprecating the deprecated option in the core library. Can you open a PR there?

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.

None yet

3 participants