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

Update Python and Flask #269

Merged
merged 21 commits into from
May 13, 2013
Merged

Update Python and Flask #269

merged 21 commits into from
May 13, 2013

Conversation

methane
Copy link
Contributor

@methane methane commented May 10, 2013

  • Update Python
  • Use hkp:// to get gpg key for environment in firewall.
  • Install tornado in installer.py
  • Add Flask-PyPy

@seanaedmiston
Copy link
Contributor

Hi,

I took a look at your pull request - I am very interested to see how PyPy works. I did however notice that in the 'Fortunes' test you have not added the additional Fortune at request time as specified (requirement #5 here: http://www.techempower.com/benchmarks/#section=code). Would be good if you could add this too.

Sean

@methane
Copy link
Contributor Author

methane commented May 13, 2013

@seanaedmiston Is it fixed?

@seanaedmiston
Copy link
Contributor

Looks like it will do the job.

One minor point. In my implementation of the 'raw' version, I found that it
performed noticeably better if instead of creating a new Fortune object
for each row, the row data was loaded into a list of tuples. (If you are
not going to use an ORM it didn't seem to make a lot of sense to create a
bunch of new objects for this simple use case.) The downside of this is
that the elements cannot be referenced by name (ie. fortune.id,
fortune.message) and so a different template is needed which references by
element (i.e. fortune[0], fortune[1]).

Cheers

On Mon, May 13, 2013 at 5:44 PM, INADA Naoki [email protected]:

@seanaedmiston https://github.com/seanaedmiston Is it fixed?


Reply to this email directly or view it on GitHubhttps://github.com//pull/269#issuecomment-17796770
.

@methane
Copy link
Contributor Author

methane commented May 13, 2013

@seanaedmiston I've misunderstand what "Within the scope of the request, a new Fortune object must be constructed and added to the list." means. I've rollbacked 77dda2a.

@bhauer
Copy link
Contributor

bhauer commented May 13, 2013

The intent of the addition of a new Fortune object during the scope of each request is:

  1. To demonstrate a dynamically-sized list/array data structure. The size needs to grow by one to make room for a newly constructed Fortune object.
  2. Demonstrate the construction of a new object.
  3. To enforce that sorting is done within the application code rather than within a ORDER BY clause in the query.

Because of number 2 above, we strongly prefer the use of a Fortune class and instance objects even if you are not using an ORM. To my eye, using an array or tuple structure seems very aggressive optimization, making it atypical for a production application. Am I wrong there?

@pfalls1
Copy link
Contributor

pfalls1 commented May 13, 2013

@methane The fortunes test is failing because there's no template fortunes.html

@methane
Copy link
Contributor Author

methane commented May 13, 2013

@pfalls1 Sorry, I've added it now.

@methane
Copy link
Contributor Author

methane commented May 13, 2013

@bhauer dbraw_engine is wrapper of raw MySQLdb (or PyMySQL) driver.
It receives tuples from underlaying driver and wraps it with new SQLAlchemy's row object.
This wrapped object provides .message property and template use it.
So I think dbraw_engine meets "creating new object" requirements.

@pfalls1 pfalls1 merged commit 7d9ee00 into TechEmpower:master May 13, 2013
@pfalls1
Copy link
Contributor

pfalls1 commented May 13, 2013

@methane Looks great, thanks!

@methane methane deleted the python-update branch May 13, 2013 15:03
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