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.

Sign up for GitHub

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

Jump to bottom

WIP: Graphene 3 #774

Closed
wants to merge 18 commits into from
Closed

WIP: Graphene 3 #774

wants to merge 18 commits into from

Conversation

ktosiek
Copy link
Contributor

@ktosiek ktosiek commented Sep 15, 2019

Related to #705. I've started working on porting graphene-django to Graphene 3.
For now I'm just fixing tests from the top, but I'm sharing this in case anyone else is intrested.

@ktosiek
Drop unsupported versions of Python and Django, and use graphql-core v3
3cd056e
@ktosiek
Fix test collection
af9e3ca
@ktosiek
Use adapters instead of Connection and PageInfo
78e510b
@ktosiek
Run DjangDebugMiddleware tests asynchronously
c77aa75
@ktosiek
Don't use __debug for DjangoDebug
4cc4add 
Fields starting with __ are reserved for introspection
@ktosiek
Register Django's _('translatable string') as a field description in …
c2ae343 
…graphql-core
@ktosiek
Upgrade tests based on schema printing
c7dea56 
graphql-core 3 prints the descriptions, and skips `schema` if it's the default one (query: Query)
@ktosiek
Fix asserts for errors that come back with visualised location
d6fd3fc
@ktosiek
Fix test_should_preserve_prefetch_related
140ad42 
Connection resolvers have access to pagination arguments.
@ktosiek
Fix rest_framework tests
8412fee
@ktosiek
Port GraphQLView to Graphene 3
5bd4d31
@ktosiek
Fix GraphQLView tests, skip some of them
d386a7e 
Some of the tests show the problem described here: graphql-python/graphql-core#61
@ktosiek
Fix async tests in test_query
1577571
@ktosiek
Drop support for django-filter < 2
ddacb9d 
It was only needed for Python 2.7
@ktosiek
Import singledispatch straight from functools
8356fcc
@jkimbo
Copy link
Member

jkimbo commented Sep 22, 2019

This is great! Thanks so much @ktosiek . Is there anything outstanding left to do?

@ktosiek
Copy link
Contributor Author

ktosiek commented Sep 23, 2019

  1. This is blocked by SourceLocation should be serialized as object graphql-core#61 (which I hope to finish soon).
  2. This was never used in an application (except for the cookbook example), so it might be incomplete.
  3. I haven't looked through documentation yet, it might still reference Python 2 or Django Filters 1.x

@ktosiek
Copy link
Contributor Author

ktosiek commented Sep 26, 2019

  1. SourceLocation should be serialized as object graphql-core#61 is done, and I've updated this PR accordingly.
  2. Still no real-world testing, and I'm not sure when I'll be able to do that (the only app I could try this on uses django-graphql-optimizer heavily).
  3. I've looked through the docs, (but not very thoroughly, to be honest), and fixed 2 references to Django <1.11.

@rhizoome rhizoome mentioned this pull request Nov 14, 2019
Closed
7 tasks
@rhizoome
Copy link
Contributor

rhizoome commented Nov 19, 2019 •
edited
Loading

@ktosiek I am going to run this against few of our projects tests.

This was referenced Nov 20, 2019
Closed
Closed
@BfutureP
Copy link

Anyone please review it ASAP. It will be helpful for my project... upgrqaded to Django v3 with Graphene v3 and Graphene-Django v3. thanks. @ #812

BfutureP
BfutureP approved these changes Dec 11, 2019
View reviewed changes
Copy link

@BfutureP BfutureP left a comment

Choose a reason for hiding this comment

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

looks good

@ulgens
Copy link
Collaborator

ulgens commented Mar 1, 2020

@ktosiek Is this still WIP?

@ktosiek
Copy link
Contributor Author

ktosiek commented Mar 1, 2020

@ulgens I'm not working on this at the moment, but I see @ganwell has taken over in a separate PR: #812.

@ulgens ulgens mentioned this pull request Mar 15, 2020
Closed
@rhizoome
Copy link
Contributor

rhizoome commented Apr 6, 2020

@jkimbo I fear that tox was not working on #904 and #905. I want to start over and incrementally (a PR and review per commit) merge #774. I also have some issues with flaky tests on master and I want to fix that first in master: Introducing random test order.

@ulgens
Copy link
Collaborator

ulgens commented Apr 6, 2020

@jkimbo Just give me some time to fix those test config. I'm saying that everything is already in place 🙂 No need for any additional work here.

@rhizoome rhizoome mentioned this pull request May 4, 2020
Merged
@jkimbo
Copy link
Member

jkimbo commented May 9, 2020

Closing this since #951 has been merged into the v3 branch

@jkimbo jkimbo closed this May 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BfutureP BfutureP BfutureP approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ktosiek @jkimbo @rhizoome @BfutureP @ulgens