Fix compatibility with graphql-core v3.2

[nikolaik]

This fixes the compatibility issues with graphql-core v3.2.

The relevant changes in graphql-core are these two commits:

• graphql-python/graphql-core@7c39e7e
• graphql-python/graphql-core@e5ba99f

Fixes #82

There was a failing relay test which I couldn't find any solution for, so I made a workaround in 5a6791d. Do you have an idea what could be wrong?

Also the promise library was failing to install on python 3.7 so I bumped the python patch version which includes a newer setuptools I believe

[codecov-commenter]

Codecov Report

Merging #83 (f18d827) into master (474bc68) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #83      +/-   ##
==========================================
+ Coverage   93.63%   93.71%   +0.07%     
==========================================
  Files           7        7              
  Lines         330      334       +4     
==========================================
+ Hits          309      313       +4     
  Misses         21       21              

Impacted Files	Coverage Δ
graphene_django_optimizer/query.py	92.10% <100.00%> (-0.03%)	⬇️
graphene_django_optimizer/utils.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 474bc68...f18d827. Read the comment docs.

[helgihg]

@tfoxy Any movement on this getting merged?

[hishamkaram]

@sicksid @tfoxy this PR is not working, i have tested it. there is no optimization applied to the queryset select_related and prefetch_related are not being added to the queryset

[helgihg]

Notifying @nikolaik of that, too.

[tfoxy]

Hi everyone, sorry for not being active here, I took some time out from coding to prioritize other things in my life.

I would like to confirm what @hishamkaram said. @nikolaik @sicksid does it optimize the queries for you? Tests are passing fine but they are not integration tests so there may be something wrong where the info being passed in the optimizer is not being actually used now when Django applies the query.

[nikolaik]

I have not been using this package in any current project, so haven't been able to prioritise this PR. I initially tried looking at the relevant changes between v3.1 and 3.2 and I believe the two linked commits in the PR description are the relevant ones. @tfoxy Do you understand why my changes are not working?

[tfoxy]

I'm not sure right now. I need to read my code again and I will try this PR with a sample app to debug.

[andrei-datcu]

Gave this a go: The PR is good and should be merged.
The observations above are unfortunately correct, but definitely not related to these changes.
The select_related optimizations are discarded currently because of https://github.com/graphql-python/graphene-django/pull/1315/files#r1106051616

To get around that particular issue, a new converter for ForeignKey/OneToOne fields may be reregistered. But again, this is not a problem related to this PR. You should merge this as is.

[sjdemartini]

(FYI as to the separate question raised above about failing query-optimization in newer graphene-django versions, I've opened a PR in graphene-django graphql-python/graphene-django#1401 which hopefully will be merged to resolve those issues.)

[firaskafri]

Fix is now available in graphene-django v3.0.2.

Just a heads up for folks who use get_queryset as an auth layer before this release. If you need that, you can stick with versions between >=3.0.0b9 and <3.1.0 for now. Feel free to suggest a different way that won't lead to those pesky N+1 SQL query issues.

[sjdemartini]

I have tested this change locally, and it worked seamlessly for my company's project that has quite a bit of graphene-django-optimizer usage, as well as for my package graphene-django-permissions.

• With graphql-core 3.1.7 installed, this branch worked fine (no change from master branch it seems) in either of the projects I tested with, as expected.

• With graphql-core 3.2.3, this branch fixed tests in both projects that were otherwise failing on the published 0.9.1 version of graphene-django-optimizer. (137 tests fixed in my company's project, 21 tests fixed in graphene-django-permissions.) Without this branch's fixes, there were errors like:

    File "/path/venv/lib/python3.10/site-packages/graphene_django_optimizer/query.py", line 54, in optimize
      field_def = get_field_def(info.schema, info.parent_type, info.field_name)
    File "/path/venv/lib/python3.10/site-packages/graphql/execution/execute.py", line 1146, in get_field_def
      field_name = field_node.name.value
  AttributeError: 'str' object has no attribute 'name'
  

I have tests that check for query optimization/performance as well, and all queries were still optimized. The select_related/prefetch_related concerns mentioned above were due to an issue in graphene-django which is fixed on v3.0.2 as mentioned above, so that is not an issue on this PR and has been resolved separately.

Would it be possible to merge and publish when you have a chance @tfoxy? Thanks so much! And thank you nikolaik for these fixes!

[salcedo]

Is this good to go using relay? I tried this and the relay query test is still failing.

[sjdemartini]

@salcedo what are you referring to by "the relay query test is still failing"? CI passed on this branch: https://github.com/tfoxy/graphene-django-optimizer/pull/83/checks. If you're referring to the test_should_return_valid_result_in_a_relay_query test, that test is failing on master (so unrelated to anything in this PR), but is actually passing here since nikolaik patched it in this branch as a workaround to get CI to pass, as he mentioned in the PR description.

[salcedo]

Yes I was unsure if that test being bypassed was a workaround for CI only or if the PR was unfinished. All appears to be working on Django 4.2.1 with Graphene 3.2.2

[sjdemartini]

Hi @tfoxy, this PR is coming up on its 1 year mark, and I definitely appreciate that you may not be able to dedicate any further time toward maintaining graphene-django-optimizer. graphene-django had a long hiatus, but has recently been revived and has been getting regular updates and improvements, so it would be a shame to have to fork graphene-django-optimizer in order to keep it up to date. I would be happy to join as a maintainer of graphene-django-optimizer if that's an option (as I also recently joined graphene-django), or perhaps you have other folks in mind who may be able to review code, merge fixes like these, and publish newer versions. (For instance, there's a pending PR in graphene-django that is likely to break some more tests here on master graphql-python/graphene-django#1411, but I'd be glad to work on and publish fixes for them.)

Thanks again for creating this package and maintaining it for so long! Let me know if there's anything else I or others can do to help.

[tfoxy]

Hi everyone! Sorry for not responding for so long. I took some time off from programming for personal reasons.

@sjdemartini your comment seems good. I'm going to make you a maintainer of this repo. Not expecting you to do any work here of course, but if it's useful to you then any contributions are welcome. Almost since creating this repo that I'm not working with Python or GraphQL, so it's kind of hard for me to understand the problems people have and how the code fixes them without having any side effects.

I'm going to merge this and release a new version since many here already used this PR successfully.

[tfoxy]

v0.10.0 is up!

Please check if it works correctly. Had to upload manually because Travis no longer works.

Thanks @nikolaik for taking the time to make the code changes and everyone else for making sure this PR works correctly.

@sjdemartini I made you a contributor. Let me know if you need anything else. I will set up GitHub Actions now so that we have testing for PRs and automatic publishing.

[nikolaik]

Thanks for taking the time @tfoxy 🤗

[sjdemartini]

@tfoxy The v0.10.0 release seems to be working for me, resolving the graphql-core v3.2 compatibility issue. Thanks again for releasing, and thank you @nikolaik for this PR!