Ticket #1208 (closed defect: fixed)

Opened 4 years ago

Last modified 3 years ago

inbox displays traceback on joule

Reported by: glyph Assigned to: exarkun
Priority: highest Milestone:
Component: Quotient Severity: blocker
Keywords: Cc:
Estimated Completion (YYYY/MM/DD): Branch:
Author:

Description

My inbox isn't displaying at all. I'll attach the traceback.

Attachments

653791612dc8aaab.html.gz (188.9 kB) - added by glyph on 06/22/06 18:51:18.

Change History

06/22/06 18:51:18 changed by glyph

  • attachment 653791612dc8aaab.html.gz added.

06/23/06 17:12:16 changed by moe

  • keywords set to review.
  • owner changed from moe to exarkun.

ready for review in inbox-rendering-recursion-1208. the branch doesn't attempt to fix the problem (because i was unable to reproduce it) but it does greatly improve test coverage for message and inbox rendering, and rendering of messages with stupid numbers of parts. also makes rendering of live fragments a bit easier to test by fixing an issue in ReliableMessageDelivery that was causing cleanup errors.

i'll leave the ticket open after i merge this.

06/23/06 19:04:55 changed by moe

i made three more changes in the branch, that do fix the problem (same problem as #1036 and #602). when Fragment.rend() creates a new context, it was setting the old context as it's parent, which was causing the size of the stack to grow exponentially against the number of fragments being rendered. everything works, AFAICT, but it's pretty difficult to make assertions about the nevow rendering code.

06/27/06 08:46:51 changed by exarkun

  • keywords deleted.

06/29/06 11:28:20 changed by exarkun

  • keywords set to review.
  • owner changed from exarkun to glyph.

Branch hijacked to implement Element and propagate it down the stack.

Ready for review in inbox-rendering-recursion-1208

06/29/06 15:02:57 changed by glyph

  • keywords deleted.
  • owner changed from glyph to exarkun.

Good:

  • Wow. A news entry. Awesome.
  • Diieeeeeeeee context dieeeeee diee fucking dieeeeee

Before merging:

  1. a trivial test failure
    [ERROR]: xquotient.test.test_rendering.RenderingTestCase.test_inboxRendering
    
    Traceback: exceptions.AttributeError: 'module' object has no attribute 'storeID'
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/twist.py:23:_drive
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/ten.py:84:iterflatten
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/flatstan.py:102:TagSerializer
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/ten.py:71:serialize
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/ten.py:62:partialflatten
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/flatstan.py:261:DirectiveSerializer
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/ten.py:71:serialize
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/ten.py:62:partialflatten
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/flat/flatstan.py:231:FunctionSerializer
    /home/glyph/Projects/Divmod/trunk/Nevow/nevow/page.py:95:<lambda>
    /home/glyph/Projects/Divmod/trunk/Quotient/xquotient/inbox.py:287:composeLink
    
  2. There should be a test case for the fix to _pysqlite2.py: that wasn't even pyflakes'ing clean and that code should definitely have been covered by a test.
  3. transacted has no docstring. Is the change to use mergeFunctionMetadata important? If so, it should be tested.
  4. no docstring on the liveform._LiveFormMixin.submitbutton renderer, and that's a template interface. It should be documented because it's sufficiently changed in this code that it shows up as all +'s for me.
  5. the docstring on _LiveFormMixin itself is wrong now, that docstring should probably be on LiveForm.
  6. LiveFormFragment is also undocumented, and given that it's legacy code, it should explain that.
  7. ValidatingSignupForm.parameterNames should be documented, or private. Probably private.
  8. No docstrings for webtheme.ThemedFragment or webtheme.ThemedFragment. Again, this should cover the distinction (i.e. note which is preferred) and have a pointer to something explaining the deprecation.
  9. util.Expose should take a docstring argument - because it doesn't, athena.expose is actually undocumented!
  10. (athena.expose needs a docstring. it's an important new API...)
  11. _LiveMixin.page needs a docstring.
  12. render_liveFragment ... needs a docstring. But it's not really new code - the real thing it needs is a deprecation warning if it's invoked on LiveElement.
  13. nevow.page's module docstring should explain a bit more, although maybe it just needs to indicate that Element is the only useful thing there.
  14. there are a bunch of new exceptions in nevow.page; every package is supposed to have a separate interfaces module and exceptions module, those should probably go in there (especially since it would be really nice if the older ways of doing things didn't have such crappy ad-hoc error reporting, before they went away entirely).
  15. Docstring for xquotient.renderers.ButtonRenderingMixin.render_button, or ButtonRenderingMixin explaining that it can now be used for either fragments or shards.
  16. xquotient.test.partmaker needs docstrings.
  17. so does the new xquotient.test.util. Why isn't PartMaker just in this module? both are ridiculously short.
  18. The docstring for INavigableFragment is now too misleading, since it specifically refers to nevow.rend.Fragment; it should be changed, and it should probably mention LiveElement rather than page.Element.

Other:

  • xquotient.renderers is reasonably testable code, I think, or at least some of it is. The amusing test-case-name should really be a ticket (assigned to Moe), not a comment. (Perhaps both.)
  • there should be tickets created to identify and update all the wiki pages and athena documents to point at LiveElement rather than LiveFragment. They should probably be assigned to community members.
  • the change to Epsilon's test_benchmark seems totally spurious. Do we have a coding-standard policy on operator.attrgetter or itemgetter? Should we?
  • also, many of the methods that are being exposed aren't documented. I don't think that should necessarily block this merge, but client->server interfaces should generally be well-documented so we can understand our security exposure.
  • no test for EditableColumnView.setter but clearly nothing was using that anyway.
  • Expose should really go in epsilon, or actually, twisted.python. It isn't politically acceptable that it do that right now, but we should probably make some tickets or something.
  • INavigableFragment and INavigableElement were confusing before, but they are insanely confusing now. There should probably be a ticket to fix that. Maybe assigned to me, I'm not sure. The only reason I didn't require a bunch of docstrings to talk about this was that everything that uses them is currently undocumented in the code :-\.
  • apropos of the second "good" point, would making a page.Page be too hard? It'd be nice to get rid of context all the way through the stack, at least in our code.

06/30/06 10:01:02 changed by exarkun

  • keywords set to review.
  • owner changed from exarkun to glyph.

Everything in the first group fixed except:

  • _LiveMixin.page already has a docstring
  • render_liveFragment can't be invoked from a LiveElement? (no render_ method can be)

For other:

  • #1249 created for testing renderers
  • #1079 already exists for updating athena wiki pages
  • The Epsilon benchmark change is semantic: .time and .getTime() return different things; .time is basically always wrong (I don't think the tests actually end up following that code path though).
  • Exposed methods should definitely be documented.
  • #1255 created for EditableColumnView?
  • Eh, I guess
  • The bulk of the work for getting rid of context completely still remains. Making page.Page wouldn't be extremely difficult, but getting the context out of nevow.flat will be.

Take a look and let me know if you think the branch is good to merge now.

07/01/06 19:16:03 changed by glyph

  • keywords deleted.
  • owner changed from glyph to exarkun.

Whew. That's a lot of code.

Yes, I think this should be merged.

As far as the work involved in removing context: I could rewrite nevow.flat from scratch, except for the zillion things that implicitly depend on context. This removal might go a long way towards allowing us to have a parallel renderer which does not preserve context, and just leave nevow.flat alone for legacy applications.

07/02/06 11:23:42 changed by exarkun

  • status changed from new to closed.
  • resolution set to fixed.

(In [7470]) Merge inbox-rendering-recursion-1208-5

Author: exarkun Reviewer: glyph Fixes #1208

Introduce the nevow.page module which primarily exposes one new class (for the moment): Element. Element is a replacement for nevow.rend.Fragment and does not expose the context to application code during the render phase, or at any other time. Athena's remote method decorator has been generalized and is now also used to expose render methods on the Element class, rather than the conventional render_ prefix.

Athena has gained LiveElement? which is just like LiveFragment? except it substitutes Element behavior in place of Fragment behavior. Athena also no longer supports the allowedMethods dictionary: expose is the only API for marking methods as remotely callable.

Mantissa and Quotient have been updated to use these new APIs in several locations. LiveForm? is now an Element rather than a Fragment. A new class for backwards compatibility has been added, LiveFormFragment?, for applications which cannot easily migrate to the requirements of Element.

Test coverage for Axiom's transacted decorator now exists. Several bugs in that function have also been fixed. Extensive test coverage for the PySQLite2 and APSW backends for Axiom has also been added and a bug in the timeout support in the PySQLite2 backend fixed. APSW was previously very broken and remains somewhat so, though with tests pointing out what exactly is broken. Timeouts now raise a specific exception, axiom.errors.TimeoutError?, rather than whatever the underlying system feels like raising.

jethro@divmod.org