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

Python 2.7 support #348

Closed
GoogleCodeExporter opened this issue Mar 28, 2015 · 29 comments
Closed

Python 2.7 support #348

GoogleCodeExporter opened this issue Mar 28, 2015 · 29 comments

Comments

@GoogleCodeExporter
Copy link

Rietveld should support Python 2.7 on App Engine both with and without 
threading.


Original issue reported on code.google.com by albrecht.andi on 2 Dec 2011 at 1:09

@GoogleCodeExporter
Copy link
Author

I've started to collect some notes on what has to be done here: 
https://docs.google.com/document/d/1_P4k0gvpBxbYZt7SSxpkrW15WW6MP1JJTQDFPnMn48I/
edit


Original comment by albrecht.andi on 2 Dec 2011 at 1:11

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

I'd split threading matters into a separate issue. It will require removing 
state shared and cached in class variables. Probably moving it into request 
object or some `Environment` object tied to request.

Original comment by techtonik@gmail.com on 10 Dec 2011 at 11:14

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

FWIW I've done the most basic port imaginable and it seems to work, although 
the /repos page is slow and I haven't turned on multi-threading.  See these two 
code reviews:

http://codereview.appspot.com/5574079/ - basic changes to app.yaml etc.
http://codereview.appspot.com/5552045/ - deal with lack of djangoforms

Original comment by gvanrossum@gmail.com on 30 Jan 2012 at 6:36

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Opening /repos page takes 6s on Python 2.5  I can create a separate issue if 
Python 2.7 will take longer.

Original comment by techtonik@gmail.com on 31 Jan 2012 at 8:24

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Looks like they're about the same speed actually, so never mind about the 
/repos page.

Original comment by gvanrossum@gmail.com on 31 Jan 2012 at 6:31

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Are we ready for the switch?

Original comment by techtonik@gmail.com on 4 Jul 2012 at 7:57

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Not yet, discussion is still here: 
https://groups.google.com/d/topic/codereview-list/fomZew3asbY/discussion

Original comment by albrecht.andi on 4 Jul 2012 at 8:07

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Sorry, that was the wrong thread. Here is the discussion: 
https://groups.google.com/d/topic/codereview-list/R14YbZU1Qdo/discussion

Original comment by albrecht.andi on 4 Jul 2012 at 8:08

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

The last remaining issue is whether we can turn on the threadsafe flag, right?  
Maybe we can just leave that off for now, but still switch to Python 2.7?  Our 
QPS is not so high that I expect we'll suffer much if we just switch to 
single-threaded Python 2.7.  We can work towards thread-safe code (mostly the 
current Account cache IIRC) and a gradual transition to NDB after the switch to 
Py27.

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 8:32

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

+1 for switching with threadsafe = off.

It should be relatively easy to change the Account model to NDB (we get caching 
for free then, right?) as it's not related to any other model.

Original comment by albrecht.andi on 4 Jul 2012 at 8:38

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Ok, I will switch today.  I have one merge to do still.  Do you mind if I push 
from the py27 branch?

Q: where should development take place going forward?  Should we merge the py27 
branch back into default or just work on the py27 branch?

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 8:46

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Ok, switched. The version ID is 'py27'.  Please watch the logs...

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 8:51

  • Changed state: Started
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

ok, will have a look at the logs!

IMO we should merge py27 into default.

Original comment by albrecht.andi on 4 Jul 2012 at 8:56

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I looked at the logs and saw some tracebacks. But they appear unrelated to the 
switch. Nevertheless, I fixed the bug and re-deployed; now the new version is 
py27a.  (Once we're comfortable with developing in Py27 only we should go back 
to numbered versions.)

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 10:39

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Right, this specific traceback was there before - I saw it yesterday, but only 
one or two times and assumed something on the JavaScript side when adding a 
patchset via the web UI (that was obviously not a good assumption). Glad you've 
fixed it!

Original comment by albrecht.andi on 4 Jul 2012 at 10:52

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

hm, now there's a different problem with the issue_wrapper...

Original comment by albrecht.andi on 4 Jul 2012 at 11:25

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

ok, it seems that everything is fine.

@guido, did you upload an unmodified version to 27a before uploading your 
change? The tracebacks are a few hours old and are not reproducible with the 
py27a version (but with the py27 version).

Original comment by albrecht.andi on 4 Jul 2012 at 11:41

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Yes, sorry, I uploaded the wrong fix, show(request, form=form), before I 
uploaded the right fix, show(request, <id>, form), to the same rev (py27a) and 
whatever user had this problem did hit the bad fix.  All should be well now!

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 12:50

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Nice job, and I am such a slowpoke. Still writing a child issue for NDB switch.

Where are the logs for the previous version 94?
`codereview` application is deprecated and empty (why keep it - the only 
historical data were logs anyway)?
`codereview-hr` version 94 contains only one log message from today. I use 
Main->Versions->Instances->View Logs.

Original comment by techtonik@gmail.com on 4 Jul 2012 at 1:02

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

> Q: where should development take place going forward?  Should we merge the 
py27 branch back into default or just work on the py27 branch?

I'd vote for branching current 'trunk' into py25, mark branch closed, merge 
py27 into mainline and continue on 'trunk' (it is tedious for me to remember to 
use `hg summary` before `hg status` every time, so I prefer not to work on 
branches in Mercurial).

Original comment by techtonik@gmail.com on 4 Jul 2012 at 1:13

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

techtonik: You're looking at the instance looks (note the prefilled "filter" 
field). Try Main -> Logs and then select the version on top.

Original comment by albrecht.andi on 4 Jul 2012 at 1:28

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Andi, if you are in agreement with tektonik's proposal for branching I will 
implement that tomorrow, assuming we don't see more new tracebacks.

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 1:33

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

IMO a tag would be sufficient (what about "future_starts_here"? :)), but I'm 
fine with a techtonik's proposal too. In both cases we should close the py27 
branch after merging into default too.

Original comment by albrecht.andi on 4 Jul 2012 at 1:51

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Thanks for the logs link. Browsing them now. Error rate seems the same with an 
average somewhere below 0.010 according to Dashboard. Dashboard doesn't 
preserve chart types in URLs, so I won't give you exact links. It is 
interesting to see on 2-days graph how AppEngine squashes values for longer 
periods.

Original comment by techtonik@gmail.com on 4 Jul 2012 at 2:19

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Ok, did the branch switch.  We are now developing using Python 2.7 in the 
default branch.

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 2:49

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Let's just close this issue.  Techtonik, didn't you say you were going to 
create a new issue to make the code threadsafe and/or for the (partial?) NDB 
port?

Original comment by gvanrossum@gmail.com on 4 Jul 2012 at 2:51

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Great! :b

Yes, I've added issue #389 for NDB support.

Original comment by techtonik@gmail.com on 4 Jul 2012 at 3:24

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

FYI, the README and wiki still needs to be updated... meanwhile, app.yaml has 
python27 on by default...

Original comment by a...@bbfdirect.com on 20 Mar 2013 at 9:56

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Thanks! I've udpated the README and the wiki pages.

Original comment by albrecht.andi on 21 Mar 2013 at 1:36

  • Added labels: ****
  • Removed labels: ****

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant