Re: Turbogears2 migration: custom error pages

[Alessandro Molina]

On Wed, Jun 29, 2016 at 9:31 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:
>
> I have pushed a rebased version to your kallithea-tg repo. I first
> marked the existing changes as obsolete, but it seems this information
> did not transfer to Bitbucket, don't know why (is your repo marked as
> non-publishing in the Settings?)


Seems that by default are created as publishing, switched it to
non-publishing so that allows draft changes.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Alessandro Molina]

On Mon, Jun 13, 2016 at 8:41 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

> Question is now: do you agree with this current split and shall I push
> it to kallithea-tg, or do you prefer I make some changes first?
>

Sorry for taking so long to reply, pretty much overwhelmed recently and saw
mail only now.
Feel free to push whenever you prefer,.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Tue, Jun 7, 2016 at 9:20 AM, Thomas De Schampheleire
 wrote:
> On Tue, Jun 7, 2016 at 12:43 AM, Alessandro Molina
>  wrote:
>>
>>
>> On Tue, May 31, 2016 at 9:02 AM, Thomas De Schampheleire
>>  wrote:
>>>
>>>
>>> If the dotted notation is more common or a better strategy, we could
>>> also just switch to it?
>>
>>
>> It has some advantages as all template paths are expressed in relation to
>> python packages, so makes easier to reuse parts of the applicaiton.
>> When using /index.html it's not clear where it should be loaded from, while
>> kallithea.templates.index explicitly tells it's the index template from
>> kallithea package.
>>
>> While TG supports templates by path, the best practice is to specify them by
>> package using the dotted notation.
>>
>>>
>>> It produces of course more changes for now, so it could be simpler to
>>> postpone this to the point where we integrated to mainstream? Or use a
>>> temporary hack to support both.
>>
>>
>> I made some minor changes to the Mako renderer in TG to improve support for
>> apps that use custom extension for template files (TG2 by default takes for
>> grnated that mako templates end with .mak extension). Using those the
>> debugbar works correctly in Kallithea, I plan to release those with TG 2.3.9
>> before during June.
>>
>
> Thanks.
>
> Meanwhile I started rebasing the current changes onto the current
> 'default' of Kallithea upstream, which also includes all my changes to
> migrate fully to pytest.
> It was not possible to do a plain rebase due to the merge that
> happened in between, so I had to use some grafting too. I collapsed
> most of the initial changes that all relate to basic bringup of TG2.
>
> I think it makes sense to continue using the kallithea-tg repo,
> obsoleting the original changes. Then everything is still available in
> one place.
>
> I'll let you know when I have something to share.
>

Ok, I have successfully rebased and squashed the commits we currently
have. The remaining patches are:

6030 057cb5938727 default  2016-06-10 Alessandro Molina Turbogears2
migration: base code changes
6041 de6e2b8f4ca8 default  2016-02-18 Alessandro Molina This should
provide a working ErrorController, still missing what it is media_path
6042 b101140fc446 default  2016-02-22 Thomas De Schampheleire
turbogears migration: remove some references to pylons from comments
6043 f673294fb6cd default  2016-02-22 Thomas De Schampheleire
turbogears migration: replace remaining references to pylons.config
6044 32add9b05a0a default  2016-02-22 Thomas De Schampheleire auth:
remove unused import of tg.url
6045 f33abfa1cc61 default  2016-04-25 Alessandro Molina Switch from
custom routing to tgext.routes
6046 5299791862e4 default  2016-06-10 Thomas De Schampheleire
turbogears migration: initial set-up of test suite
6047 5287ae164712 default  2016-05-17 Thomas De Schampheleire
turbogears migration: tests: fix tests using internationalization
6048 f784f584aa78 default  2016-05-29 Thomas De Schampheleire
turbogears migration: make sure 400 errors use the custom error pages
6049 d7399eefadd3 tip default  2016-06-13 Thomas De Schampheleire
Turbogears2 migration: tests: other: fix tests outside of controllers

The first commit (6030) is basically most of the initial commits by
Alessandro but squashed because I think they were more development
iterations than commits that should remain separately.
I also squashed 3 commits related to the switch to tgext.routes into
1, but kept it separately from the first one.
I kept the commit for ErrorController separately, mainly to highlight
Alessandro's comment 'still missing what it is media_path'.
Later we could squash more commits, e.g. my commits fixing tests,
depending on how that proceeds.

Question is now: do you agree with this current split and shall I push
it to kallithea-tg, or do you prefer I make some changes first?

Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Tue, Jun 7, 2016 at 12:43 AM, Alessandro Molina
 wrote:
>
>
> On Tue, May 31, 2016 at 9:02 AM, Thomas De Schampheleire
>  wrote:
>>
>>
>> If the dotted notation is more common or a better strategy, we could
>> also just switch to it?
>
>
> It has some advantages as all template paths are expressed in relation to
> python packages, so makes easier to reuse parts of the applicaiton.
> When using /index.html it's not clear where it should be loaded from, while
> kallithea.templates.index explicitly tells it's the index template from
> kallithea package.
>
> While TG supports templates by path, the best practice is to specify them by
> package using the dotted notation.
>
>>
>> It produces of course more changes for now, so it could be simpler to
>> postpone this to the point where we integrated to mainstream? Or use a
>> temporary hack to support both.
>
>
> I made some minor changes to the Mako renderer in TG to improve support for
> apps that use custom extension for template files (TG2 by default takes for
> grnated that mako templates end with .mak extension). Using those the
> debugbar works correctly in Kallithea, I plan to release those with TG 2.3.9
> before during June.
>

Thanks.

Meanwhile I started rebasing the current changes onto the current
'default' of Kallithea upstream, which also includes all my changes to
migrate fully to pytest.
It was not possible to do a plain rebase due to the merge that
happened in between, so I had to use some grafting too. I collapsed
most of the initial changes that all relate to basic bringup of TG2.

I think it makes sense to continue using the kallithea-tg repo,
obsoleting the original changes. Then everything is still available in
one place.

I'll let you know when I have something to share.

/Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Alessandro Molina]

On Tue, May 31, 2016 at 9:02 AM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

>
> If the dotted notation is more common or a better strategy, we could
> also just switch to it?
>

It has some advantages as all template paths are expressed in relation to
python packages, so makes easier to reuse parts of the applicaiton.
When using /index.html it's not clear where it should be loaded from, while
kallithea.templates.index explicitly tells it's the index template from
kallithea package.

While TG supports templates by path, the best practice is to specify them
by package using the dotted notation.


> It produces of course more changes for now, so it could be simpler to
> postpone this to the point where we integrated to mainstream? Or use a
> temporary hack to support both.
>

I made some minor changes to the Mako renderer in TG to improve support for
apps that use custom extension for template files (TG2 by default takes for
grnated that mako templates end with .mak extension). Using those the
debugbar works correctly in Kallithea, I plan to release those with TG
2.3.9 before during June.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Mon, May 30, 2016 at 11:10 PM, Alessandro Molina
 wrote:
>
>
> On Sun, May 29, 2016 at 9:19 PM, Thomas De Schampheleire
>  wrote:
>>
>>
>> > Currently, paster is still used. I briefly tried using gearbox (the
>> > supposed replacement) but it did not work. I did not spend more time
>> > on that to focus on the rest, but it should be tackled at some point.
>
>
> Switching to gearbox should be relatively easy, I'll try to check the reason
> in the next days.
>
>>
>> > Other stuff like 'debugbar' also seems very useful, but did not work
>> > out of the box either.
>
>
> Issue is that the debugbar, like most TG applications, uses the dotted
> notation for template files.
> Which in Kallithea is disabled because kallithea renders templates as
> '/filename.html' instead of 'kallithea.filename'.
>
> Using the debugbar would require the dotted template engine resolution,
> which can't be enabled because the mako renderer in TG doesn't support
> mixing the two syntaxes.
> Other renderers like the genshi one are smarter and allow mixing path and
> dotted notations inside same project, but as the mako was one of the less
> used on TurboGears it is not as refined as the others.
>
> I can probably fix this by subclassing the mako renders in Kallithea and
> providing a custom renderer that supports both syntaxes.
>

If the dotted notation is more common or a better strategy, we could
also just switch to it?
It produces of course more changes for now, so it could be simpler to
postpone this to the point where we integrated to mainstream? Or use a
temporary hack to support both.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Alessandro Molina]

On Sun, May 29, 2016 at 9:19 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

>
> > Currently, paster is still used. I briefly tried using gearbox (the
> > supposed replacement) but it did not work. I did not spend more time
> > on that to focus on the rest, but it should be tackled at some point.
>

Switching to gearbox should be relatively easy, I'll try to check the
reason in the next days.


> > Other stuff like 'debugbar' also seems very useful, but did not work
> > out of the box either.
>

Issue is that the debugbar, like most TG applications, uses the dotted
notation for template files.
Which in Kallithea is disabled because kallithea renders templates as
'/filename.html' instead of 'kallithea.filename'.

Using the debugbar would require the dotted template engine resolution,
which can't be enabled because the mako renderer in TG doesn't support
mixing the two syntaxes.
Other renderers like the genshi one are smarter and allow mixing path and
dotted notations inside same project, but as the mako was one of the less
used on TurboGears it is not as refined as the others.

I can probably fix this by subclassing the mako renders in Kallithea and
providing a custom renderer that supports both syntaxes.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Tue, May 24, 2016 at 7:13 PM, Mads Kiilerich  wrote:
> On 05/23/2016 08:34 PM, Thomas De Schampheleire wrote:
>>
>> This rebase could also incur some squashing of patches, to make it more
>> ready for upstreaming. What do you think of that?
>
>
> I haven't checked the repo recently, but it seems like there has been quite
> a bit of experimenting and going back and forth. It would probably be nice
> to get it reworked and cleaned up so we avoid adding unnecessary "debt" from
> the beginning. Just guessing.
>
> I understand TurboGears is very modular and uses existing libraries. Now,
> with a almost-already-successful more-than-proof-of-concept, could it make
> sense to migrate the existing application to these libraries one by one? For
> example starting with replacing paster?
>

I don't know if it will be that easy, at least I don't see the
modularity right now in the current codebase.

Currently, paster is still used. I briefly tried using gearbox (the
supposed replacement) but it did not work. I did not spend more time
on that to focus on the rest, but it should be tackled at some point.
Other stuff like 'debugbar' also seems very useful, but did not work
out of the box either.
Surely all these are small issues that can be fixed with minor changes.

While actual usage seems fine, there are still many issues in the test
suite and I think we should resolve them before trying to go mainline.

/Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Mads Kiilerich]

On 05/23/2016 08:34 PM, Thomas De Schampheleire wrote:
This rebase could also incur some squashing of patches, to make it 
more ready for upstreaming. What do you think of that?


I haven't checked the repo recently, but it seems like there has been 
quite a bit of experimenting and going back and forth. It would probably 
be nice to get it reworked and cleaned up so we avoid adding unnecessary 
"debt" from the beginning. Just guessing.


I understand TurboGears is very modular and uses existing libraries. 
Now, with a almost-already-successful more-than-proof-of-concept, could 
it make sense to migrate the existing application to these libraries one 
by one? For example starting with replacing paster?


/Mads
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Tue, May 24, 2016 at 8:38 AM, Alessandro Molina
 wrote:
> You probably just have to add 400 to errorpage.status_codes option in config
>
> See
> http://turbogears.readthedocs.io/en/latest/reference/config-options.html#tg.appwrappers.errorpage.ErrorPageApplicationWrapper
>

Indeed, thanks, added these lines:

diff --git a/kallithea/config/app_cfg.py b/kallithea/config/app_cfg.py
--- a/kallithea/config/app_cfg.py
+++ b/kallithea/config/app_cfg.py
@@ -78,6 +78,9 @@ base_config.DBSession = kallithea.model.
 # Configure App without an authentication backend.
 base_config.auth_backend = None

+# Configure ErrorPage behavior
+base_config['errorpage.status_codes'] = [400, 401, 403, 404]
+
 try:
 # Enable DebugBar if available, install tgext.debugbar to turn it on
 from tgext.debugbar import enable_debugbar
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

Hi Alessandro,

On Fri, Feb 19, 2016 at 10:14 PM, Thomas De Schampheleire
 wrote:
> On Thu, Feb 18, 2016 at 4:23 PM, Alessandro Molina
>  wrote:
>>
>>
>> On Thu, Feb 18, 2016 at 4:13 PM, Thomas De Schampheleire
>>  wrote:
>>>
>>>
>>> So this means that Kallithea would start using the 'default' routing
>>> method used by Turbogears projects?
>>> Is this a big impact / does it require a lot of work, according to you?
>>
>>
>> Nope, just that instead of relying RoutesMiddleware to run mapper.routematch
>> and fill environ with routes variables (routes.url, routes.route,
>> wsgiorg.routing_args) those should be filled by RootController itself.
>>
>> Actually as I'm moving the routing logic from Kallithea to tgext.routes (so
>> that Kallithea can just use it instead of implementing custom routing
>> itself) that change is already undergoing, but I didn't have time to finish
>> it so far.
>>
>>
>
> Thanks, that sounds good...
>
> Meanwhile I can confirm that my scenario with custom error pages work fine 
> now.
>

Found a case where the custom error pages do not work, e.g. with this change:

diff --git a/kallithea/controllers/home.py b/kallithea/controllers/home.py
--- a/kallithea/controllers/home.py
+++ b/kallithea/controllers/home.py
@@ -67,6 +67,7 @@ class HomeController(BaseController):
 #json used to render the grid
 c.data = json.dumps(repos_data)

+raise HTTPBadRequest()
 return render('/index.html')

 @LoginRequired()


and loading the home page, causes a plain page with text
"400 Bad Request
The server could not comply with the request since it is either
malformed or otherwise incorrect."

I verified that the same change on original Kallithea has the custom
error page correctly displayed...

Ideas?



Meanwhile, the pytest migration has finished. I am planning to
rebasing our current codebase on the latest Kallithea upstream to
avoid making changes on obsolete code. Especially since I'm seeing
many test failures with Turbogears2, and I expect that at least in
some cases the test themselves should be fixed.
This rebase could also incur some squashing of patches, to make it
more ready for upstreaming. What do you think of that? And shall I
push the result to your bitbucket repo or rather to another one?

Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Thu, Feb 18, 2016 at 4:23 PM, Alessandro Molina
 wrote:
>
>
> On Thu, Feb 18, 2016 at 4:13 PM, Thomas De Schampheleire
>  wrote:
>>
>>
>> So this means that Kallithea would start using the 'default' routing
>> method used by Turbogears projects?
>> Is this a big impact / does it require a lot of work, according to you?
>
>
> Nope, just that instead of relying RoutesMiddleware to run mapper.routematch
> and fill environ with routes variables (routes.url, routes.route,
> wsgiorg.routing_args) those should be filled by RootController itself.
>
> Actually as I'm moving the routing logic from Kallithea to tgext.routes (so
> that Kallithea can just use it instead of implementing custom routing
> itself) that change is already undergoing, but I didn't have time to finish
> it so far.
>
>

Thanks, that sounds good...

Meanwhile I can confirm that my scenario with custom error pages work fine now.

/Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Alessandro Molina]

On Thu, Feb 18, 2016 at 12:05 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:
>
> Thanks, for basic 404 it indeed works.
> However, I discovered at least one scenario where it does not:
>

Thanks for pointing that out.
The reason is that when dispatching errors we do not pass through the
middleware stack again, so RoutesMiddleware is not executed again and the
routes dict remains the same as before. This causes our RootController to
dispatch it again to the same route as before which leads to a 404 error
again.

I provided a work-around in
https://bitbucket.org/_amol_/kallithea-tg/commits/5ec7001cd47dece91580d8d533bacf3f0ab0d990
but probably the "correct" solution would be to remove RoutesMiddleware and
perform the route resolution inside RootController so that the route is
resolved every time we perform a dispatch and not only when receiving a
request from the WSGI server.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: custom error pages

[Thomas De Schampheleire]

On Thu, Feb 18, 2016 at 11:24 AM, Alessandro Molina
 wrote:
>
>
> On Thu, Feb 18, 2016 at 10:56 AM, Alessandro Molina
>  wrote:
>>
>>
>>
>> On Thu, Feb 18, 2016 at 10:26 AM, Thomas De Schampheleire
>>  wrote:
>>>
>>> More ideas?
>>>
>>
>> I'll try to check it today before lunch.
>
>
> https://bitbucket.org/_amol_/kallithea-tg/commits/7e27305d248627d2e15bb2e8bdc3dbc84065c094
> seems to fix the custom error pages.
>

Thanks, for basic 404 it indeed works.
However, I discovered at least one scenario where it does not:
1. create a new empty repository (Admin->Repositories->Add repository)
2. from the new page, click Options -> Fork, accept default settings
3. from the forked repo page, click Options -> Compare Fork

In the original Pylons kallithea, a custom error page with a flash is
shown, with the solution above it is a real 404.
The url is:
http://localhost:5000/empty/compare/r...@tip...rev@tip?other_repo=empty-fork=1


> Sadly I wrongly pulled from mainstream repository, so I also brought in all
> the changes from the current main kallithea repository, while I just wanted
> to pull your changes from the kallithea-tg repo. They merged without
> conflicts, but I don't know if something broke.

Content-wise it's probably fine.
I guess we'll need to cleanup commits before being able to integrate
it completely in Kallithea mainstream, so it's not a big problem I
think.

Thanks,
Thomas
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general