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

support Django 2 + in tests #336

Merged
merged 8 commits into from
Jun 5, 2018
Merged

Conversation

urbandove
Copy link
Contributor

@urbandove urbandove commented Dec 6, 2017

Foreign keys in Django 2 require an on_delete attribute
Also the field.rel attribute has been deprecated as of Django 2 and is now field.remote_field
I have added a fallback for 1.8

The django docs specifying the changes to field.rel

@coveralls
Copy link

coveralls commented Dec 6, 2017

Coverage Status

Coverage decreased (-0.6%) to 92.528% when pulling d314d10 on urbandove:django2 into 24706f5 on graphql-python:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 92.347% when pulling 384bff0 on urbandove:django2 into 24706f5 on graphql-python:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 92.347% when pulling 384bff0 on urbandove:django2 into 24706f5 on graphql-python:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 92.347% when pulling 384bff0 on urbandove:django2 into 24706f5 on graphql-python:master.

@urbandove
Copy link
Contributor Author

I should note that in my previous pull request #335 i added on_delete attribute to all related fields - as this is a required field starting in 2.0 (but supported before)

@coveralls
Copy link

coveralls commented Dec 6, 2017

Coverage Status

Coverage increased (+0.02%) to 93.163% when pulling 489d287 on urbandove:django2 into 24706f5 on graphql-python:master.

@urbandove
Copy link
Contributor Author

Have updated python version in setup.py so that if its python 2 it doesnt try install django 2
this makes build pass - which hasnt been happening till now

@urbandove urbandove changed the title rel -> remote_field: Update Django DeprecatedAttribute support Django 2 + in tests Dec 7, 2017
@coveralls
Copy link

coveralls commented Dec 7, 2017

Coverage Status

Coverage increased (+0.02%) to 93.163% when pulling f687406 on urbandove:django2 into 24706f5 on graphql-python:master.

@coveralls
Copy link

coveralls commented Dec 7, 2017

Coverage Status

Coverage increased (+0.02%) to 94.799% when pulling 43c0c99 on urbandove:django2 into a480a39 on graphql-python:master.

@urbandove
Copy link
Contributor Author

Now the 2.0 tests are failing because pytest-django has not released a version which supports 2.0 (Coming in version 3.1.3)
(Also -the current pytest-django version is pinned at an old version anyways)
We can wait for the new version to be push or pip install directly from the github master branch

@spockNinja
Copy link
Contributor

Thanks for your work here! It's looking great so far.

It looks like pytest-django 3.1.3 is coming pretty soon, so my vote would be to wait for that and then we can update the test_requires to unpin/update it.

If you want to try out their master in the meantime to make sure it doesn't break anything else, that might be a good next move. There's always a chance that updating to their master will cause older python/django versions to break.

@spockNinja
Copy link
Contributor

For now, I'm going to steal this bit so we can at least get new PRs passing their builds.

django_version = 'Django>=1.8.0,<2' if sys.version_info[0] < 3 else 'Django>=1.8.0'

@spockNinja spockNinja mentioned this pull request Dec 9, 2017
@spockNinja
Copy link
Contributor

I cherry-picked your 489d287 commit into #338 so we can get other builds passing again.

@urbandove
Copy link
Contributor Author

Hi @spockNinja
pytest-django finally updated their pypi version to 3.2.1 which supports django2. So now this patch is ready to go.
There is the field.remote field change from an if-else to a try-catch - which I think is the accepted way for dealing with deprecated fields.

@syrusakbary syrusakbary merged commit 883d177 into graphql-python:master Jun 5, 2018
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.

4 participants