[PATCH] backout "pullrequests: don't add automatic 'status change' message - it will be added in template"

Jan Heylen heyleke at gmail.com
Wed Jun 17 01:40:42 EDT 2015


What I also wanted to say is that the original change is most probably
not the issue, it just triggers an underlying issue...

What I observed:
our kallithea repo:
* the issue is still seen on the tip of the default branch
* the issue is not seen on the stable branch (as this commit is not on
the stable branch)
inside kallithea:
* the issue is only seen on commits that are in a (freshly created) pull request
* the issue is only seen when those commits don't have any other
comments given yet

Btw, that is why I still believe we need to backout the breaking
change, any commit that is in a fresh created pull request without any
comments on it, cannot be accessed.

My 5 cents in solving it:
* It is not the first time I was thinking this: a for loop over a an
object that is a NoneType should not go wrong in this way. I think
this is wrong in the mako runtime. I had the same issue in the email
templates. But I could be wrong? A simple workaround though is to
place an extra %if statement to check for NoneType...
* The actual question is, how is c.comments a noneType in this case,
and not when there is no review request? That could answer the above
question, I am wrong and c.comments is created because a PR is open,
but 0 comments are returned, so, the first entry on c.comments stays a
NoneType, that would be an issue in the code that creates c.comments.
But still, the for loop should not loop over an empty array?

I had a brief look at the changelog for mako, but couldn't find a
direct reference to it.
http://docs.makotemplates.org/en/latest/changelog.html

I hope this info is useful,

br,

Jan

On Wed, Jun 17, 2015 at 6:56 AM, Jan Heylen <heyleke at gmail.com> wrote:
> hg update -r 14d75d4b03cd
> paster setup-db development.ini
> paster serve ...
>
> 1) http://localhost:5000/test-review/settings/advanced
> --> set test-review a fork of test (test-review has 1 one extra commit)
>
> 2) http://localhost:5000/test-review/changelog
> --> click open a new pull request
> --> fill in a title
> --> click create pull request
> --> now pr is created, in pull request content overview, click on the
> commit id link
> (e.g. http://localhost:5000/test-review/changeset/9fc53eb55523722f7f23bc8e355c449d50cae6bc)
>
> the resulting output:
>
>
> Error !
>
> AttributeError: 'NoneType' object has no attribute 'comment_id'
>
> 150
>
>     </div>
>
> 151
>
> 152
>
>     %for co in c.comments:
>
> 153
>
>         <div id="comment-tr-${co.comment_id}">
>
> 154
>
>           ${comment_block(co)}
>
> 155
>
>         </div>
>
> 156
>
>     %endfor
>
> 157
>
> </div>
>
> 158
>
> </%def>
>
> traceback:
>
> /URL: http://localhost:5000/test-review/changeset/9fc53eb55523722f7f23bc8e355c449d50cae6bc
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/WebError-0.10.3-py2.7.egg/weberror/evalexception.py',
> line 431 in respond
>   app_iter = self.application(environ, detect_start_response)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Beaker-1.6.4-py2.7.egg/beaker/middleware.py',
> line 155 in __call__
>   return self.wrap_app(environ, session_start_response)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Routes-1.13-py2.7.egg/routes/middleware.py',
> line 131 in __call__
>   response = self.app(environ, start_response)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/wsgiapp.py',
> line 107 in __call__
>   response = self.dispatch(controller, environ, start_response)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/wsgiapp.py',
> line 312 in dispatch
>   return controller(environ, start_response)
> File '/home/jan/src/kallithea-allu-upstream/kallithea/lib/base.py',
> line 383 in __call__
>   return WSGIController.__call__(self, environ, start_response)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/controllers/core.py',
> line 211 in __call__
>   response = self._dispatch_call()
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/controllers/core.py',
> line 162 in _dispatch_call
>   response = self._inspect_call(func)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/controllers/core.py',
> line 105 in _inspect_call
>   result = self._perform_call(func, args)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/controllers/core.py',
> line 57 in _perform_call
>   return func(**args)
> File '<string>', line 2 in index
> File '/home/jan/src/kallithea-allu-upstream/kallithea/lib/auth.py',
> line 782 in __wrapper
>   return func(*fargs, **fkwargs)
> File '<string>', line 2 in index
> File '/home/jan/src/kallithea-allu-upstream/kallithea/lib/auth.py',
> line 841 in __wrapper
>   return func(*fargs, **fkwargs)
> File '/home/jan/src/kallithea-allu-upstream/kallithea/controllers/changeset.py',
> line 325 in index
>   return self._index(revision, method=method)
> File '/home/jan/src/kallithea-allu-upstream/kallithea/controllers/changeset.py',
> line 313 in _index
>   return render('changeset/changeset.html')
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/templating.py',
> line 243 in render_mako
>   cache_type=cache_type, cache_expire=cache_expire)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/templating.py',
> line 218 in cached_template
>   return render_func()
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Pylons-1.0-py2.7.egg/pylons/templating.py',
> line 240 in render_template
>   return literal(template.render_unicode(**globs))
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/template.py',
> line 452 in render_unicode
>   as_unicode=True)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/runtime.py',
> line 803 in _render
>   **_kwargs_for_callable(callable_, data))
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/runtime.py',
> line 835 in _render_context
>   _exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/runtime.py',
> line 855 in _exec_template
>   _render_error(template, context, compat.exception_as())
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/runtime.py',
> line 864 in _render_error
>   result = template.error_handler(context, error)
> File '/home/jan/src/kallithea-venv/local/lib/python2.7/site-packages/Mako-1.0.0-py2.7.egg/mako/runtime.py',
> line 853 in _exec_template
>   callable_(context, *args, **kwargs)
> File '/home/jan/src/kallithea-allu-upstream/data/templates/base/root.html.py',
> line 209 in render_body
>   __M_writer(escape(next.body()))
> File '/home/jan/src/kallithea-allu-upstream/data/templates/base/base.html.py',
> line 42 in render_body
>   __M_writer(escape(next.main()))
> File '/home/jan/src/kallithea-allu-upstream/data/templates/changeset/changeset.html.py',
> line 269 in render_main
>   __M_writer(escape(comment.generate_comments()))
> File '/home/jan/src/kallithea-allu-upstream/data/templates/changeset/changeset_file_comment.html.py',
> line 245 in render_generate_comments
>   __M_writer(escape(co.comment_id))
> AttributeError: 'NoneType' object has no attribute 'comment_id'
>
> On Wed, Jun 17, 2015 at 1:36 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 06/16/2015 08:31 PM, Jan Heylen wrote:
>>>
>>> # HG changeset patch
>>> # User Jan Heylen <heyleke at gmail.com>
>>> # Date 1434479490 -7200
>>> #      Tue Jun 16 20:31:30 2015 +0200
>>> # Node ID 3ff99546a950144258be3078b4483e7cebd5d720
>>> # Parent  53d68f201e4602d3f2ccfcd27107d2ebea2deea2
>>> backout "pullrequests: don't add automatic 'status change' message - it
>>> will be added in template"
>>>
>>> this commit causes following simple test to fail:
>>> * start from empty database with 2 repos, one a fork of the other, 1
>>> commit diff
>>> * create a new pull request with that one commit
>>> * after the pull request is created, immediatly click on the link to the
>>> commit
>>>    in the pull request content list
>>> --> error with undefined comments variable
>>
>>
>> I cannot reproduce the problem.
>>
>> Exactly which URL are you clicking?
>> Exactly what error do you get?
>> I assume you get a stack trace on the server - which one?
>>
>> /Mads
>>
>>>
>>> diff -r 53d68f201e46 -r 3ff99546a950 kallithea/model/pull_request.py
>>> --- a/kallithea/model/pull_request.py   Mon Jun 08 06:46:48 2015 +0200
>>> +++ b/kallithea/model/pull_request.py   Tue Jun 16 20:31:30 2015 +0200
>>> @@ -93,7 +93,7 @@
>>>           #reset state to under-review
>>>           from kallithea.model.comment import ChangesetCommentsModel
>>>           comment = ChangesetCommentsModel().create(
>>> -            text=u'',
>>> +            text=u'Auto status change to %s' %
>>> (ChangesetStatus.get_status_lbl(ChangesetStatus.STATUS_UNDER_REVIEW)),
>>>               repo=org_repo,
>>>               user=new.author,
>>>               pull_request=new,
>>> _______________________________________________
>>> kallithea-general mailing list
>>> kallithea-general at sfconservancy.org
>>> http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
>>
>>


More information about the kallithea-general mailing list