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

Upgrade Django to 1.11 [One Step at a time] #135

Merged
merged 5 commits into from
Apr 12, 2020
Merged

Upgrade Django to 1.11 [One Step at a time] #135

merged 5 commits into from
Apr 12, 2020

Conversation

aktech
Copy link
Member

@aktech aktech commented Apr 10, 2020

Upgrading everything to latest version in one go is not a great idea and will be hard to review as well, I am planning to upgrading things slowly and one by one so that the diff is not a lot.

  • This PR upgrades Django (1.3 -> 1.11)
  • Minor pep8 changes
  • Update copyright to 2020 (making it automatic is one for later)
  • Tests Pass

@asmeurer @certik @lidavidm

templates/base.html Outdated Show resolved Hide resolved
@asmeurer
Copy link
Member

@prshnt19 has done some work with Django for SymPy Live sympy/sympy-live#144. Perhaps you can review each other's work.

I can also give you access to push up to the App Engine if you need it.

@aktech aktech changed the title Upgrade Django to 1.11 [One Step at a time] [WIP] Upgrade Django to 1.11 [One Step at a time] Apr 10, 2020
Co-Authored-By: Aaron Meurer <asmeurer@gmail.com>
@aktech
Copy link
Member Author

aktech commented Apr 10, 2020

@asmeurer Sure, sounds great!

@aktech aktech changed the title [WIP] Upgrade Django to 1.11 [One Step at a time] Upgrade Django to 1.11 [One Step at a time] Apr 10, 2020
@aktech
Copy link
Member Author

aktech commented Apr 11, 2020

@prshnt19 Can you please have a look at this PR?

except DeadlineExceededError:
return HttpResponse(json.dumps({
'error': 'Computation timed out.'
}), mimetype="application/json")
}), content_type="application/json")

Choose a reason for hiding this comment

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

I think its better to use JsonResponse.

Copy link
Member Author

Choose a reason for hiding this comment

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

Beyond the scope, See this

@@ -381,12 +382,14 @@ def remove_query(request, qid):
'message': 'Not logged in or invalid user.'
}

return HttpResponse(json.dumps(response), mimetype='application/json')
return HttpResponse(json.dumps(response), content_type='application/json')

Choose a reason for hiding this comment

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

Suggested change
return HttpResponse(json.dumps(response), content_type='application/json')
return JsonResponse(response)

Copy link
Member Author

Choose a reason for hiding this comment

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

Beyond the scope, See this

settings.py Outdated
}]


ALLOWED_HOSTS = ['127.0.0.1', 'localhost']
Copy link

@prshnt19 prshnt19 Apr 12, 2020

Choose a reason for hiding this comment

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

Allowed hosts should contain the domain 'https://sympygamma.com' for production.

@@ -26,6 +26,8 @@ handlers:

libraries:
- name: django
version: "1.3"
version: "1.11"
- name: numpy
version: "1.6.1"

Choose a reason for hiding this comment

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

Suggested change
version: "1.6.1"
version: "1.18.1"

I think numpy should be updated to latest.

Copy link
Member Author

Choose a reason for hiding this comment

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

Beyond the scope, See this

Copy link

@prshnt19 prshnt19 left a comment

Choose a reason for hiding this comment

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

@aktech I think instead of Python2 we should use Python3. Python2 support is already dropped for SymPy.

@aktech
Copy link
Member Author

aktech commented Apr 12, 2020

@aktech I think instead of Python2 we should use Python3. Python2 support is already dropped for SymPy.

@prshnt19 Thanks for the review. This is beyond the scope of this PR. The plan is to do incremental working updates. See this comment. This PR only contains minimum amount of changes required to support Django 1.11, following PR could contain upgrading further versions incrementally.

@prshnt19
Copy link

Well, I only have doubts for python2. Other than that changes look fine to me

@aktech
Copy link
Member Author

aktech commented Apr 12, 2020

Well, I only have doubts for python2. Other than that changes look fine to me

This is an attempt to prepare for Python3 upgrade. It will happen incrementally.

@asmeurer
Copy link
Member

SymPy Gamma is also hosted at gamma.sympy.org

@aktech
Copy link
Member Author

aktech commented Apr 12, 2020

SymPy Gamma is also hosted at gamma.sympy.org

Updated ALLOWED_HOSTS and deployed here:
https://sympy-gamma-aktech.uc.r.appspot.com/input/?i=x

@aktech aktech merged commit 200679c into sympy:master Apr 12, 2020
@aktech aktech added the GAE-Python3 Everything related to upgrading to Google App engine's Python3 runtime label Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GAE-Python3 Everything related to upgrading to Google App engine's Python3 runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants