Begin porting from Python2/Django 1.11 to Python3/Django 2.0 #70

speizerj · 2017-12-03T06:32:31Z

Changes the Dockerfile to use Python 3.6, and requirements.txt to use Django 2.0

Clean up exceptions to Python3 format in dashboard/helpers.py

Changes the Dockerfile to use Python 3.6, and requirements.txt to use Django 2.0 Clean up exceptions to Python3 format in dashboard/helpers.py

owocki · 2017-12-03T16:44:57Z

awesome! a few questions:

mind resolving the conflicts?
whats left TODO here?
should i ask the community to test this yet?

speizerj · 2017-12-03T17:09:56Z

mind resolving the conflicts?

yep, will do

whats left TODO here?

I need to get the app to a point where it can run the local server successfully before we can test. I'll have a better time estimate after working on it some more today.

should i ask the community to test this yet?

hold off for now, once the app is running then I'll need the help

Changing the requirements.txt file to simply "PyPDF2" may have done the trick here. PyPDF hasn't been actively maintained for years and it appears that PyPDF2 has ported it to Python3 successfully.

DRF needed to be upgraded to be compatible with Django 2.0, and the sitemaps module just needed some basic tweaking to get working.

speizerj · 2017-12-03T19:51:00Z

@owocki the local server is now running successfully on python3/django2. I've tested out the lifecycle of a funded issue on Ropsten and it's working for me (funding, claiming, accepting, rejecting)

Tips appear to be broken and so that's what I'll work on next, but if anyone has any insight there I'm all ears. The request.body is empty in send_tip_2 view, not sure why.

Now would be a good time to open it up to testing - I'm sure there are plenty of edge cases.

mbeacom · 2017-12-04T04:13:05Z

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:2.7-slim
+FROM python:3.6


Would it be possible to switch this to 3.6-slim-jessie ?

owocki · 2017-12-04T05:40:34Z

just posted about this to #focus-dev-python on slack.

also, looks like theres a bunch new merge conflicts. might be because of the isort PR. sorry!

mbeacom · 2017-12-04T05:57:57Z

My apologies! Most, if not all of those conflicts should be fine to resolve if you drop in the setup.cfg and run sort -rc --atomic . in the repo root: web @speizerj

owocki · 2017-12-04T06:03:59Z

installing the dependancies so i can test now..

owocki · 2017-12-04T06:06:52Z

installing the dependancies so i can test now..

owocki · 2017-12-04T06:11:15Z

got it running and most pages seem to work; did not test the flows end to end but i similarly get the tips error

mbeacom · 2017-12-04T06:24:33Z

The failure on send_tip_2 is because b'' is being passed as request.body and falsely passing through if request.body != '': since it's true.
Not saying this is the right answer, but if request.body not in ['', b'']: at least eliminates the JSONDecodeError

merging in latest changes

…on:3.6-slim-jessie Docker image Merge branch 'master' of https://github.com/speizerj/web into python3

speizerj · 2017-12-04T07:19:20Z

dockerfile has been updated and merge conflicts resolved manually - working on the tip bug now

Changed the send/receive tip views to check for response.body rather than comparing equality to an emtpy string

speizerj · 2017-12-04T07:54:43Z

Tips appear to be working now, though I could only test out the sending functionality. The receive page appears to work as well but I couldn't confirm because I don't have the proper setup to test out the email functionality, and I couldn't get a link to test.

jasonrhaas

Just a few minor comments.

jasonrhaas · 2017-12-04T16:12:38Z

Dockerfile

@@ -1,12 +1,12 @@
-FROM python:2.7-slim
+FROM python:3.6-slim-jessie


Any reason you chose the -slim-jessie image over the -slim or just the regular python:3.6 image? I'm wondering if some of the linux packages you are installing below might already be included in one of the other python images. Might speed up the build time a bit.

Using the slim base cuts this docker image size in half... The slim base for 3.6 defaults to stretch, which is unsupported by a dependency we use due to the openssl version at the moment.

Right -- I usually use the slim versions also unless I have some issue installing things on the container. Makes sense then re: openssl version.

jasonrhaas · 2017-12-04T16:14:58Z

app/dashboard/helpers.py

@@ -67,7 +67,7 @@ def title(request):
    urlVal = URLValidator()
    try:
        urlVal(url)
-    except ValidationError, e:
+    except ValidationError as e:


Total nit pick but if you aren't using any information from the Error Message you shouldn't even need the as e part.

jasonrhaas · 2017-12-04T16:15:15Z

app/dashboard/helpers.py

@@ -77,7 +77,7 @@ def title(request):

    try:
        html_response = requests.get(url)
-    except ValidationError, e:
+    except ValidationError as e:


Same comment as above.

jasonrhaas · 2017-12-04T16:15:22Z

app/dashboard/helpers.py

@@ -96,7 +96,7 @@ def title(request):
            for link in soup.find_all('h1'):
                print(link.text)

-    except ValidationError, e:
+    except ValidationError as e:


Same comment as above.

jasonrhaas · 2017-12-04T16:15:32Z

app/dashboard/helpers.py

@@ -119,7 +119,7 @@ def keywords(request):
    urlVal = URLValidator()
    try:
        urlVal(url)
-    except ValidationError, e:
+    except ValidationError as e:


Same comment as above.

jasonrhaas · 2017-12-04T16:15:39Z

app/dashboard/helpers.py

@@ -138,7 +138,7 @@ def keywords(request):
        keywords.append(split_repo_url[-2])

        html_response = requests.get(repo_url)
-    except ValidationError, e:
+    except ValidationError as e:


Same comment as above.

jasonrhaas · 2017-12-04T16:15:44Z

app/dashboard/helpers.py

@@ -149,7 +149,7 @@ def keywords(request):
        for ele in eles:
            keywords.append(ele.text)

-    except ValidationError, e:
+    except ValidationError as e:


Same comment as above.

jasonrhaas · 2017-12-04T16:16:14Z

app/dashboard/views.py

@@ -53,7 +53,7 @@ def send_tip(request):
 @ratelimit(key='ip', rate='2/m', method=ratelimit.UNSAFE, block=True)
 def receive_tip(request):

-    if request.body != '':
+    if request.body:


owocki · 2017-12-04T16:19:11Z

thanks for pushing this fwd yall. heads down on other stuff today, but hit me up on slack if you want more community involvement

merging in latest from master

Since byte object doesn't have a format method, use % formatting instead, per: https://stackoverflow.com/a/22701750

speizerj · 2017-12-07T07:09:12Z

FYI here's the error I'm getting when trying to submit the /whitepaper form (looks to be sendgrid related - assuming it's lack of credentials):

Traceback:

File "/usr/local/lib/python3.6/site-packages/python_http_client/client.py" in _make_request
  157.             return opener.open(request)

File "/usr/local/lib/python3.6/urllib/request.py" in open
  532.             response = meth(req, response)

File "/usr/local/lib/python3.6/urllib/request.py" in http_response
  642.                 'http', request, response, code, msg, hdrs)

File "/usr/local/lib/python3.6/urllib/request.py" in error
  570.             return self._call_chain(*args)

File "/usr/local/lib/python3.6/urllib/request.py" in _call_chain
  504.             result = func(*args)

File "/usr/local/lib/python3.6/urllib/request.py" in http_error_default
  650.         raise HTTPError(req.full_url, code, msg, hdrs, fp)

During handling of the above exception (HTTP Error 401: Unauthorized), another exception occurred:

File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py" in inner
  35.             response = get_response(request)

File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  128.                 response = self.process_exception_by_middleware(e, request)

File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  126.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/usr/local/lib/python3.6/site-packages/ratelimit/decorators.py" in _wrapped
  30.             return fn(*args, **kw)

File "/code/app/tdi/views.py" in whitepaper_new
  76.     send_mail(settings.CONTACT_EMAIL, settings.CONTACT_EMAIL, "New Whitepaper Request", str(body))

File "/code/app/marketing/mails.py" in send_mail
  62.     response = sg.client.mail.send.post(request_body=mail.get())

File "/usr/local/lib/python3.6/site-packages/python_http_client/client.py" in http_request
  227.                 return Response(self._make_request(opener, request))

File "/usr/local/lib/python3.6/site-packages/python_http_client/client.py" in _make_request
  161.             raise exc

Exception Type: UnauthorizedError at /whitepaper
Exception Value: HTTP Error 401: Unauthorized

owocki · 2017-12-07T16:21:57Z

you can generate a free sendgrid key at sendgrid.com (or just comment out that line for testing)

…ring generation The whitepaper PDF generation did not work in Python3. The majority of changes were porting from using file() to open(). For bytestring generation - I've changed the format to use string.encode() instead of coercing the string to a bytestring with b"".

speizerj · 2017-12-07T22:15:52Z

Whitepaper generation is now working for me locally, though I had to test with a dummy PDF (I don't have the whitepaper source). The watermark is being generated but does appear to be cut off a bit on the right hand side - not sure if that's a new bug or existing.

allow_tags has been deprecated (see https://docs.djangoproject.com/en/dev/internals/deprecation/#deprecation-removed-in-2-0) Using format_html and mark_safe instead.

owocki · 2017-12-11T20:19:11Z

looks like theres a few merge conflicts on this PR now. we should go ahead and get this merged so that the overhead of keeping it current is less :P

owocki · 2017-12-11T20:19:29Z

@ speizerj thanks for your help on this so far. i can do a final round of QA and hopefully we can get this out by mid week

speizerj · 2017-12-11T23:10:30Z

no worries @owocki I don't mind keeping it current - I'll take care of those merge conflicts tonight.

Sounds good re: QA/merge - let me know how I can help

mbeacom · 2017-12-11T23:28:41Z

I'm going to be raising a PR for GH integration and bounty model changes this evening, so I should be able to run through this again at some point shortly. Anything specific that I should hit or just run down the views and hit some of the common methods? I was hoping to get in some general tests to help you out, but didn't get that far yet! Maybe after the GH/bounty changes! Also, sorry for the conflicts! 😛

owocki · 2017-12-18T23:16:48Z

derp. looks like theres some merge conflcits now..

i could likely carve out some time to nail this down on weds if anyone else wants to do that with me

speizerj · 2017-12-18T23:18:44Z

sure thing - I'll take care of the conflicts tonight and then happy to join you on Wed

mbeacom · 2017-12-19T02:20:44Z

We should be good with GH/express interest before then, so I should be free to help you both test/resolve whatever is needed as well (and you'll probably have more merge conflicts )

Let me know if either of you need any help with this before then!

The tinyurl library is not python3 compatible, so I've migrated to the pyshortener library.

jasonrhaas · 2018-01-09T04:08:03Z

hows it going on the move from Python 2 --> 3?

owocki · 2018-01-10T01:10:30Z

its... fallen to the backburner unforgtunately... :(

owocki · 2018-01-16T23:46:33Z

closing this as it's been moved over to the january2018_feature_integration branch. @mbeacom @speizerj are there any ongoing topics of converstaion from this branch you want to carry over there?

speizerj · 2018-01-16T23:59:30Z

Nope I think we should be good, the only outstanding items were keeping it current with regards to merge conflicts and testing out all the flows to ensure there were no issues. I'll follow up on the new PR

gitcoinbot · 2018-02-01T00:07:54Z

This issue now has a funding of 0.03 ETH (33.65 USD) attached to it.

If you would like to work on this issue you can claim it here.
If you've completed this issue and want to claim the bounty you can do so here
Questions? Get help on the Gitcoin Slack
$12461.63 more Funded OSS Work Available at: https://gitcoin.co/explorer

owocki · 2018-02-01T00:08:52Z

whoops.. meant to post this to another issue.. cleaning this up now

gitcoinbot · 2018-02-01T15:07:21Z

The funding of 0.03 ETH (33.36 USD) attached to this issue has been killed by the bounty submitter

Questions? Get help on the Gitcoin Slack
$12275.84 more Funded OSS Work Available at: https://gitcoin.co/explorer

leaderboard clientside jscript version that just needs thegraph fetch

Begin porting from Python2 to Python3 and Django 2.0

385c6dc

Changes the Dockerfile to use Python 3.6, and requirements.txt to use Django 2.0 Clean up exceptions to Python3 format in dashboard/helpers.py

speizerj mentioned this pull request Dec 3, 2017

move to python3, django 2.0 #68

Closed

speizerj added 7 commits December 3, 2017 11:01

python3 default encoding is UTF8, so removing deprecated code

5ef2772

Using urllib.parse

3ca4bdf

Removing all sys.reload calls, as UTF8 is default on py3

e9d3b98

Porting PyPDF to PyPDF2

c46e379

Changing the requirements.txt file to simply "PyPDF2" may have done the trick here. PyPDF hasn't been actively maintained for years and it appears that PyPDF2 has ported it to Python3 successfully.

Updating DRF to 3.7.3, fixing sitemaps module

b2dc975

DRF needed to be upgraded to be compatible with Django 2.0, and the sitemaps module just needed some basic tweaking to get working.

Merge branch 'master' into python3

72c4cb1

Merge branch 'master' into python3

4e93db0

mbeacom reviewed Dec 4, 2017

View reviewed changes

speizerj added 3 commits December 3, 2017 22:53

Merge pull request #1 from gitcoinco/master

91dfa12

merging in latest changes

Merging in lastest changes from gitcoin/web:master, switching to pyth…

a24ba4f

…on:3.6-slim-jessie Docker image Merge branch 'master' of https://github.com/speizerj/web into python3

Merging in dockerfile changes

e597980

Fixing Tip bugs

c7a796a

Changed the send/receive tip views to check for response.body rather than comparing equality to an emtpy string

jasonrhaas reviewed Dec 4, 2017

View reviewed changes

speizerj added 3 commits December 4, 2017 10:26

cleaning up exception handling

fb11645

Merge branch 'master' into python3

f63e2d3

Merge pull request #2 from gitcoinco/master

5e4d4c1

merging in latest from master

Using % formatting for byte string

f26067b

Since byte object doesn't have a format method, use % formatting instead, per: https://stackoverflow.com/a/22701750

speizerj added 2 commits December 7, 2017 14:06

Reverting back to string.format() now that we're using string.encode()

d58173c

Fixing admin links

5cc9502

allow_tags has been deprecated (see https://docs.djangoproject.com/en/dev/internals/deprecation/#deprecation-removed-in-2-0) Using format_html and mark_safe instead.

speizerj added 2 commits December 11, 2017 21:12

Merging upstream/master back in to get current

435236e

Merge branch 'master' into python3

8df780b

speizerj added 3 commits December 19, 2017 12:21

resolving merge conficts

75376ec

merging

37a3f34

Using pyshortener instead of tinyurl

bc6da1b

The tinyurl library is not python3 compatible, so I've migrated to the pyshortener library.

mbeacom added the upgrade label Dec 31, 2017

mbeacom mentioned this pull request Jan 15, 2018

(WIP) January 2018 Multiple Integrations #240

Merged

3 tasks

owocki closed this Jan 16, 2018

owocki pushed a commit that referenced this pull request Apr 30, 2021

Merge pull request #70 from nopslip/richard-demo-implementation

0b4f6cd

leaderboard clientside jscript version that just needs thegraph fetch

		@@ -1,12 +1,12 @@
		FROM python:2.7-slim
		FROM python:3.6-slim-jessie

Begin porting from Python2/Django 1.11 to Python3/Django 2.0 #70

Begin porting from Python2/Django 1.11 to Python3/Django 2.0 #70

Conversation

speizerj commented Dec 3, 2017

owocki commented Dec 3, 2017

speizerj commented Dec 3, 2017 • edited

speizerj commented Dec 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owocki commented Dec 4, 2017

mbeacom commented Dec 4, 2017

owocki commented Dec 4, 2017

owocki commented Dec 4, 2017

owocki commented Dec 4, 2017 • edited

mbeacom commented Dec 4, 2017

speizerj commented Dec 4, 2017

speizerj commented Dec 4, 2017

jasonrhaas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbeacom Dec 4, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owocki commented Dec 4, 2017

speizerj commented Dec 7, 2017

owocki commented Dec 7, 2017

speizerj commented Dec 7, 2017

owocki commented Dec 11, 2017

owocki commented Dec 11, 2017

speizerj commented Dec 11, 2017

mbeacom commented Dec 11, 2017 • edited

owocki commented Dec 18, 2017

speizerj commented Dec 18, 2017

mbeacom commented Dec 19, 2017

jasonrhaas commented Jan 9, 2018

owocki commented Jan 10, 2018

owocki commented Jan 16, 2018

speizerj commented Jan 16, 2018

gitcoinbot commented Feb 1, 2018

owocki commented Feb 1, 2018

gitcoinbot commented Feb 1, 2018

speizerj commented Dec 3, 2017 •

edited

owocki commented Dec 4, 2017 •

edited

mbeacom Dec 4, 2017 •

edited

mbeacom commented Dec 11, 2017 •

edited