Add support for Graphene3 and update lint with black

[sebsasto]

This PR address issue #53

I added support for graphene and graphene-django v3. I also used black to improve the formatting of the code.

However with this change it looks like the only versions of Django supported are > 3

All the test pass. Please let me know if we can merge it or you need more info.

[wodCZ]

I appreciate the work done here (will try this PR in a minute).

Just a little criticism here - I really wanted to review the changes here, but fixing the support AND changing code style of unrelated code in one commit (important one too) makes it pretty painful.

@sebsasto would you reconsider changing the code style? Don't take me wrong - I'm glad someone took the effort to add Graphene v3 support and you have my 💯 , I just believe that for the sake of review-ability and git history keeping this should not be merged together.

EDIT: Just tested this on a simple app against django==3.1.6 & graphene-django==3.0.0b7 and everything seems to be working fine.

[sebsasto]

Hey @wodCZ yes I realized that I changed the code style without a commit when I was going to push the code changes.

I’m really sorry about that. I remember most of the changes I did so I can try to move them into a single branch without the style changes.

Unfortunately I will have time only next week to do this!

Apologies for that IDK what happened to me. Maybe a little bit tired 🥲

[wodCZ]

No need to be sorry, you did a great job (it's working great 🙂 ).

To be a bit of help, I checked why the build is failing. It seems that graphql-python 3 dropped the support for older Python versions and only supports 3.6+ now.

Since there's nothing this lib can do about it, I believe we can change the travis config to better match the graphql one.

.travis.yml:

dist: xenial
language: python
python:
  - "3.9"
  - "3.8"
  - "3.7"
  - "3.6"
env:
  - DJANGO_VERSION=">=3.1.0,<3.2"
  - DJANGO_VERSION=">=3.0.0,<3.1"
  - DJANGO_VERSION=">=2.2.0,<2.3"
  - DJANGO_VERSION=">=2.1.0,<2.2"
  - DJANGO_VERSION=">=2.0.0,<2.1"
  - DJANGO_VERSION=">=1.11.0,<2.0.0"
install:
  - pip install -r dev-env-requirements.txt
  - pip install "django$DJANGO_VERSION"
  - pip install codecov
script:
  - ./setup.py test
after_success:
  - codecov
jobs:
  include:
    - stage: lint
      install:
        - pip install flake8==3.7.7
      script:
        - flake8
    - stage: deploy
      if: tag IS present
      install: true
      script: skip
      deploy:
        provider: pypi
        user: tfoxy-bot
        password:
          secure: K2Qh5rx1tEoOc28W7y4sYyja/QK/L8HxNjtZjwV/5cb/3GbPckzFAUAz5YdpBU+VYvOCllRcACNq/vpJN4gCmIti9gbPIUEHeBkSxEvhG91KNqO0OLplkM9AaUQP14xLxYi/tdP0twtbMNleR8FAt350KUrxCQOvt6QMuTeVErNv4bGD+QW6nPh6caeY/2Zfm5YWDQrehLfVjlthxkozxBaqlWQzXQqY80VjQ4q77JkQ0kCRVcuDUgvIsn1wGh1Sdfoegu9Grp5yQisMW7b9BkTrFaFtZasLg+SuW81quMeA8390AUiKdjS3LF2FnjetxG52VhBEnonw8JbaVr/ZLVcYtd0CChF9AxUdriJ80i7S+9SvI4JGNtG8Vdr0B8JdDklXcAycu+/tMynAqPjmRFROK3vpqMBdFfi2GO71ZECcXAuJbj30ED30MOxlPRAZMuoLmLuADrO2wH9g7h/p51XaCdnvhE+HN/gx+0Aw2e2qieD6qBEaT10UA0J3nppymmLNwUdVqTlcnjIdaLqZTKPdNSS2SWeEcvrc9Y0R0F28nQbXUvNX6stgJKSzn+zL/v31BSwIOPOKFCY5vp0fORL2zTQ/PmwmHZeOedEniphL7yrP5+UJTMnaY3l6CpwMUhabVJFkIm3jz62mohzhe7k5pe/D0x3wz+fq8gQ4vqg=
        on:
          tags: true
stages:
  - test
  - lint
  - deploy

setup.py:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import os
import sys
from setuptools import setup

needs_pytest = {'pytest', 'test', 'ptr'}.intersection(sys.argv)
pytest_runner = ['pytest-runner >=4.0,<5dev'] if needs_pytest else []

def read(fname):
    return open(os.path.join(os.path.dirname(__file__), fname)).read()

setup(
    name='graphene-django-optimizer',
    version='1.0.0',
    author='Tomás Fox',
    author_email='tomas.c.fox@gmail.com',
    description='Optimize database access inside graphene queries.',
    license='MIT',
    keywords='graphene django optimizer optimize graphql query prefetch select related',
    url='https://github.com/tfoxy/graphene-django-optimizer',
    packages=['graphene_django_optimizer'],
    setup_requires=pytest_runner,
    long_description=read('README.md'),
    long_description_content_type='text/markdown',
    classifiers=[
        'Development Status :: 4 - Beta',
        'Environment :: Web Environment',
        'Intended Audience :: Developers',
        'License :: OSI Approved :: MIT License',
        'Operating System :: OS Independent',
        'Programming Language :: Python :: 3',
        'Programming Language :: Python :: 3.6',
        'Programming Language :: Python :: 3.7',
        'Programming Language :: Python :: 3.8',
        'Programming Language :: Python :: 3.9',
        'Framework :: Django',
        'Framework :: Django :: 1.11',
        'Framework :: Django :: 2.0',
        'Framework :: Django :: 2.1',
        'Framework :: Django :: 2.2',
        'Framework :: Django :: 3.0',
        'Framework :: Django :: 3.1',
    ],
)

The testing matrix could be extended to support python 3.9 - I expect there's nothing to break there.

Thing to consider would be to add these changes while keeping support for graphene 2, but given that this lib is not very actively updated (thus there will not be many changes to backport when needed), I'm in for merging this without backwards compatibility and releasing as a new major release.

[Rainshaw]

hello, any update on this?

[wodCZ]

@RainshawGao looks like @tfoxy is busy? I was using this PR successfully for some while, but my project was cancelled and never made it to production. There might've been a case where the optimizer didn't get through some fields (don't remember what case that was), but still it was much better than without it.

❯ cat requirements.in
pip-tools
Django~=3.1
uwsgi~=2.0
psycopg2-binary~=2.8
graphene-django~=3.0.0b7
whitenoise~=5.2
django-cors-headers~=3.7
twilio~=6.52
colorlog~=4.7.2
django-filter~=2.4.0
https://github.com/sebsasto/graphene-django-optimizer/tarball/c2248e7

[Rainshaw]

@wodCZ thanks for your reply, I will have a try with this branch....

[felixmeziere]

Hi there, thank you for this PR! :)

Ive been using this branch in production for 2 weeks (at www.greengo.voyage), what's the status on getting it merged?

[ulgens]

Wouldn't it be better if this is 2 seperate PRs? I have no maintainer rights on this repo but personally i'd reject it because it creates a dirty history in the repo.

[ulgens]

Hey @wodCZ yes I realized that I changed the code style without a commit when I was going to push the code changes.

I’m really sorry about that. I remember most of the changes I did so I can try to move them into a single branch without the style changes.

Unfortunately I will have time only next week to do this!

I can help with this.

[tfoxy]

Hi everyone! Thanks @sebsasto for taking the time to do the PR. As other commenters said, it would be better to split this in two PRs. Maybe the first one can be only formatting and hopefully this one can be rebased after the formatting one is merged. I think it's a good idea to have a lint tool for the repo.

@wodCZ thanks for proposing the changes for CI. I agree with "merging this without backwards compatibility and releasing as a new major release". Once formatting PR is created and merged then we can do that with this change.

@ulgens I would appreciate if you can help us splitting the PR. I will gladly review/merge.

It's been a long long time since I coded with python, so I'm a little rusty in the review side, but seeing as this PR has some people already using it successfully, it makes it easier to know that it doesn't seem to break anything.

Sorry for not paying attention to this PR. Personal stuff got in the way. But luckily I can now dedicate time to this.

[felixmeziere]

I've been using this in prod successfully for 3 months at https://www.greengo.voyage

[wodCZ]

Hi @tfoxy, glad to hear you're fine :)

I see @ulgens has already made the linting PR. Are you planning on making the compatibility PR too? I have a vacation next week, so I'll have some free time to look into that if not.

[ulgens]

@wodCZ My plan was to create several different PRs on styling, version updates on CI and setup, and finally graphene 3.

[ulgens]

This is ready to rebase now, but i can't do it on the original branch sebsasto:master because I'm not a maintainer. @sebsasto @tfoxy Is it okay to fork it then create another PR from my repo?

[tfoxy]

From my side it's good me it's good, unless @sebsasto is active to do it. If there's no answer from his side for today, feel free to fork it and create PR tomorrow. I would also like to have travis fixed before merging the PR for this. Luckily tomorrow I have the day free so I'll have it fixed by this time tomorrow.

[tfoxy]

Just wanted to mention that travis is fixed, so any new PR should have the CI checks.

[ldynia]

@tfoxy Can you remove support for python 2.7 so this way CI/CD will pass ? travis

[surajpatildev]

Any update on this?

[ulgens]

Ready to review under https://github.com/tfoxy/graphene-django-optimizer/pull/72

[tfoxy]

72 was merged, so closing this! Thanks everyone for contributing here. You can now use v0.9.0 for graphene 3 support.