#6431 Upgrade to ming 0.4.x to avoid extraneous count() queries

v1.0.1
closed
nobody
performance (126) 42cc (432)
General
Tim Van Steenburgh
2015-08-20
2013-07-01
Dave Brondsema
No

Ming 0.4 removes the __len__ method which does a count() query. This is so that list(query) and query.all() don't cause that to run any more.

We'll need to find & change all our len(query) and query|length (in templates) to query.count().

Related

Tickets: #6431

Discussion

  • Dave Brondsema

    Dave Brondsema - 2013-09-03
    • labels: performance --> performance, 42cc
    • summary: Upgrade to ming 0.4 to avoid extraneous count() queries --> Upgrade to ming 0.4.x to avoid extraneous count() queries
    • Description has changed:

    Diff:

    --- old
+++ new
@@ -1,5 +1,3 @@
 Ming 0.4 removes the `__len__` method which does a count() query.  This is so that list(query) and query.all() don't cause that to run any more.

 We'll need to find & change all our `len(query)` and `query|length` (in templates) to `query.count()`.
-
-If we're not comfortable with finding these manually via test runs, searching, and exercising all parts of Allura, we could potentially add support in Ming to inspect the frame within `__len__` and if it's not a call from `list()` then raise a DeprecationWarning.  (We can't simply raise DeprecationWarning all the time since list(query) automatically calls `__len__` under the covers)
     

  • Igor Bondarenko - 2013-09-04
    • status: open --> in-progress
     

  • Igor Bondarenko - 2013-09-04

    Created #430: [#6431] Upgrade to ming 0.4.x to avoid extraneous count() queries (4cp)

     

    Related

    Tickets: #6431

  • Igor Bondarenko - 2013-09-23
    • status: in-progress --> code-review
     

  • Igor Bondarenko - 2013-09-23

    Closed #430. je/42cc_6431

    Seems like all done, but there is one test failing, though:

    ======================================================================
ERROR: forgediscussion.tests.functional.test_forum.TestForumStats.test_stats_data
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jetmind/.virtualenvs/allura/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/jetmind/.virtualenvs/allura/local/lib/python2.7/site-packages/mock.py", line 1193, in patched
    arg = patching.__enter__()
  File "/home/jetmind/.virtualenvs/allura/local/lib/python2.7/site-packages/mock.py", line 1268, in __enter__
    original, local = self.get_original()
  File "/home/jetmind/.virtualenvs/allura/local/lib/python2.7/site-packages/mock.py", line 1242, in get_original
    "%s does not have the attribute %r" % (target, name)
AttributeError: <class 'ming.session.Session'> does not have the attribute 'aggregate'

    Seems like Session.aggregate was removed in Ming 0.4.1, but I see that it was added back in this commit. So, tests pass with Ming installed from current masters, and fail with 0.4.1 from PyPi. I don't sure how you want to handle this, maybe create a dev release on PyPi from current master or disable this test until new release on Ming.

     

  • Tim Van Steenburgh - 2013-09-23
    • QA: Tim Van Steenburgh
    • Milestone: forge-backlog --> forge-oct-04
     

  • Tim Van Steenburgh - 2013-09-23

    We'll cut a new release from Ming master, so you can run the tests against that.

    Changes in Allura look good, but please also review forge-classic and sftheme and make the necessary changes there too.

     

  • Tim Van Steenburgh - 2013-09-23
    • status: code-review --> in-progress
     

  • Igor Bondarenko - 2013-09-24

    Created #445: [#6431] Make forge-classic and sftheme compatible with Ming 0.4.x (2cp)

     

    Related

    Tickets: #6431

  • Igor Bondarenko - 2013-09-25
    • status: in-progress --> code-review
     

  • Igor Bondarenko - 2013-09-25

    Closed #445. Reviewed forge-classic and sftheme code. No changes here, all looks compatible with ming 0.4.1.

     

  • Dave Brondsema

    Dave Brondsema - 2013-09-25

    Additional changes on allura:db/6431 and merciless:db/bool

     

  • Tim Van Steenburgh - 2013-09-26
    • status: code-review --> blocked
     

  • Tim Van Steenburgh - 2013-09-26

    Pushed a new commit to allura:db/6431

    This is gtg, but neither allura nor ming branches have been merged. Tested with ming:db/bool (rebased to master) and allura:db/6431.

     

  • Dave Brondsema

    Dave Brondsema - 2013-10-01
    • status: blocked --> closed
     

Log in to post a comment.