Re: Turbogears2 migration: API calls

[Thomas De Schampheleire]

On Sun, Sep 18, 2016 at 12:23 PM, Alessandro Molina
 wrote:
>
>
> On Sat, Jul 30, 2016 at 10:01 PM, Thomas De Schampheleire
>  wrote:
>>
>> Hi,
>>
>> API calls currently do not work under Turbogears2.
>>
>> The simplest call is:
>>
>> ../venv/kallithea-tg-clean/bin/kallithea-api
>> --apikey=811a4ad6f382e75c20392b668cad9408bde9f42e
>> --apihost=http://localhost:5000 test
>>
>> (where apikey can be obtained from the web UI, after logging in, going
>> to 'My account' via you user name top right, then API keys.)
>
>
> Hi Thomas,
>
> I made a proof of concept of working APIs, I tried to keep it to minimum
> changes so it suffers of `self.something=` issues but seems it's able to
> receive requests, respond and report errors.
>
> See it at
> https://bitbucket.org/_amol_/kallithea-tg/commits/ead627a07f0c30961772985a441d9ef574c4b061
>
> It will require tgext.routes>=0.2.0 which I changed to allow mixing multiple
> routing systems as required by Kallithea RPC APIs
>

I applied this patch on top of the latest stack, tried a few API
calls, and indeed it seems to work fine. All API tests also pass now.

So things look very good now, thanks a lot!

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

Re: Turbogears2 migration: API calls

[Alessandro Molina]

On Sat, Jul 30, 2016 at 10:01 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

> Hi,
>
> API calls currently do not work under Turbogears2.
>
> The simplest call is:
>
> ../venv/kallithea-tg-clean/bin/kallithea-api
> --apikey=811a4ad6f382e75c20392b668cad9408bde9f42e
> --apihost=http://localhost:5000 test
>
> (where apikey can be obtained from the web UI, after logging in, going
> to 'My account' via you user name top right, then API keys.)
>

Hi Thomas,

I made a proof of concept of working APIs, I tried to keep it to minimum
changes so it suffers of `self.something=` issues but seems it's able to
receive requests, respond and report errors.

See it at
https://bitbucket.org/_amol_/kallithea-tg/commits/ead627a07f0c30961772985a441d9ef574c4b061

It will require tgext.routes>=0.2.0 which I changed to allow mixing
multiple routing systems as required by Kallithea RPC APIs
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: API calls

[Alessandro Molina]

On Mon, Aug 29, 2016 at 10:44 AM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

> >
> >   - Routing, Kallithea is probably the only TG2 app using routes as the
> core
> > dispatch system. This is probably not going away unless all controllers
> are
> > rewritten using ObjectDispatch. But as using tgext.routes as the main
> > dispatch system should be possible I tend to think that Kallithea should
> not
> > be forced to change this and I can work to fix any problems that arise.
>
> To use ObjectDispatch, we'd need extensive usage of the _lookup method
> inside the controller, is that correct?
> http://turbogears.readthedocs.io/en/latest/turbogears/
> objectdispatch.html#the-lookup-method


Most controllers are just dispatched based on their method name, so it
shouldn't be required.
For REST like uris ( /collection/ID/action ) or that dispatch based on HTTP
method those can be satisfied by subclassing from RestController

But again, I think this shouldn't be required as using tgext.routes as the
main dispatch system must work :D
So I'll try to fix any dispatch related issue we might find.


> >   - Assigning to **self**, this is pretty widespread in Kallithea
>
> Can you give examples from the Kallithea source code? In the
> controllers I only find three files that have assignments to self:
> feed.py, api/__init__.py, journal.py.
>

I noticed in BaseController that it's done in a few places:

 self.cut_off_limit = safe_int(config.get('cut_off_limit'))
 self.sa = meta.Session
 self.scm_model = ScmModel(self.sa)

And as you stated also in API is done.

I remember I created some properties to work-around such behaviour and
provide backward compatibility from code that read ip_addr and authuser

@property
def ip_addr(self):
return c.ip_addr

@property
def authuser(self):
return c.authuser

but many more are probable set around the code.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: API calls

[Thomas De Schampheleire]

On Mon, Aug 29, 2016 at 10:15 AM, Alessandro Molina
 wrote:
>> So the routing tries to access an index method.
>>
>> In the API code, the JSONRPCController parent class of ApiController
>> is supposed to direct the calls to the correct method, but it seems we
>> do not enter that class.
>>
>
> JSONRPCController needs to be able to implement his own routing and
> tgext.routes had support for mounting a regex routed controller inside an
> ObjectDispatch controller but didn't provide support for the opposite
> (mounting a subcontroller inside a regex routed controller). So it didn't
> support dispatching further from the result of the regex match. (which ended
> up in looking for index as it reached the ApiController and not further).
>
> I managed to get it dispatch to the right method, but still need to write
> tests for the tgext.routes changes and check that the whole API tests pass,
> so it will probably be a few days before I can push something decent. And it
> will surely need review by someone that knew how JSONRPCController worked as
> I'll probably miss some expected behaviours.

Thanks!

>
>>
>> I have seen in the TG2 documentation that typical, recommended, usage
>> is different than what Kallithea is doing, and I think we should
>> investigate if we can adapt our code base to such best practices
>> eventually, but in the short term I think we should try to make the
>> existing code base work.
>
>
> There are three major things that Kallithea does that are different from
> usual TG apps:

Thanks for this list, it is very useful to get such an 'audit'.

>
>   - Routing, Kallithea is probably the only TG2 app using routes as the core
> dispatch system. This is probably not going away unless all controllers are
> rewritten using ObjectDispatch. But as using tgext.routes as the main
> dispatch system should be possible I tend to think that Kallithea should not
> be forced to change this and I can work to fix any problems that arise.

To use ObjectDispatch, we'd need extensive usage of the _lookup method
inside the controller, is that correct?
http://turbogears.readthedocs.io/en/latest/turbogears/objectdispatch.html#the-lookup-method

>
>   - Expositions, currently it calls render explicitly at end of each
> controller, but the suggested and preferred way in TurboGears is using
> @expose and let TG do the template rendering, caching and template choice
> based on content_type. Most should work also by manually calling rendering,
> but kallithea might face problems due to this in the future as it is in fact
> skipping the whole Decoration/Exposition and TG (or its extensions) expect
> it to be always available for actions.
>
>   - Assigning to **self**, this is pretty widespread in Kallithea and is an
> absolute no-no in TG. All controllers in TG are allocated per process, so
> assigning anything to controllers leads to race conditions when served in a
> multithreaded environment and even in single threaded envs it might leads to
> spurious data from previous requests. Data should only be assigned to
> tg.request or tg.tmpl_context so all self.something should be properties
> that read/write from on of those two places.

Can you give examples from the Kallithea source code? In the
controllers I only find three files that have assignments to self:
feed.py, api/__init__.py, journal.py.

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

Re: Turbogears2 migration: API calls

[Alessandro Molina]

>
> So the routing tries to access an index method.
>
> In the API code, the JSONRPCController parent class of ApiController
> is supposed to direct the calls to the correct method, but it seems we
> do not enter that class.
>
>
JSONRPCController needs to be able to implement his own routing and
tgext.routes had support for mounting a regex routed controller inside an
ObjectDispatch controller but didn't provide support for the opposite
(mounting a subcontroller inside a regex routed controller). So it didn't
support dispatching further from the result of the regex match. (which
ended up in looking for index as it reached the ApiController and not
further).

I managed to get it dispatch to the right method, but still need to write
tests for the tgext.routes changes and check that the whole API tests pass,
so it will probably be a few days before I can push something decent. And
it will surely need review by someone that knew how JSONRPCController
worked as I'll probably miss some expected behaviours.


> I have seen in the TG2 documentation that typical, recommended, usage
> is different than what Kallithea is doing, and I think we should
> investigate if we can adapt our code base to such best practices
> eventually, but in the short term I think we should try to make the
> existing code base work.


There are three major things that Kallithea does that are different from
usual TG apps:

  - Routing, Kallithea is probably the only TG2 app using routes as the
core dispatch system. This is probably not going away unless all
controllers are rewritten using ObjectDispatch. But as using tgext.routes
as the main dispatch system should be possible I tend to think that
Kallithea should not be forced to change this and I can work to fix any
problems that arise.

  - Expositions, currently it calls render explicitly at end of each
controller, but the suggested and preferred way in TurboGears is using
@expose and let TG do the template rendering, caching and template choice
based on content_type. Most should work also by manually calling rendering,
but kallithea might face problems due to this in the future as it is in
fact skipping the whole Decoration/Exposition and TG (or its extensions)
expect it to be always available for actions.

  - Assigning to **self**, this is pretty widespread in Kallithea and is an
absolute no-no in TG. All controllers in TG are allocated per process, so
assigning anything to controllers leads to race conditions when served in a
multithreaded environment and even in single threaded envs it might leads
to spurious data from previous requests. Data should only be assigned to
tg.request or tg.tmpl_context so all self.something should be properties
that read/write from on of those two places.
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Re: Turbogears2 migration: API calls

[Alessandro Molina]

On Mon, Aug 22, 2016 at 8:06 PM, Thomas De Schampheleire <
patrickdeping...@gmail.com> wrote:

> Hi Alessandro,
>
> This is a gentle reminder in case you missed this mail,
>

Sorry, yep I missed that, have been a few superbusy months at work and I
have been unable to follow open source projects as much as I wanted.
I promise I'll take a look during the weekend! :D

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

Re: Turbogears2 migration: API calls

[Mads Kiilerich]

On 07/30/2016 10:01 PM, Thomas De Schampheleire wrote:

line 119, in __call__
 response = self._perform_call(context)
   File 
"/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/controllers/dispatcher.py",
line 99, in _perform_call
 state = self._get_dispatchable(context, py_request.quoted_path_info)
   File 
"/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/controllers/dispatcher.py",
line 73, in _get_dispatchable
 state = state.resolve()
   File 
"/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/crank/dispatchstate.py",
line 178, in resolve
 return self._root_dispatcher._dispatch(self, self._path)
   File 
"/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tgext/routes/dispatch.py",
line 146, in _dispatch
 action = getattr(controller, action_name)
AttributeError: 'ApiController' object has no attribute 'index'


So the routing tries to access an index method.


This reminds me of 
https://kallithea-scm.org/repos/kallithea/changeset/1fdff0d0516c ... but 
it doesn't seem to be directly related, only in spirit.


/Mads


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

Re: Turbogears2 migration: API calls

[Thomas De Schampheleire]

Hi Alessandro,

On Sat, Jul 30, 2016 at 10:01 PM, Thomas De Schampheleire
 wrote:
> Hi,
>
> API calls currently do not work under Turbogears2.
>
> The simplest call is:
>
> ../venv/kallithea-tg-clean/bin/kallithea-api
> --apikey=811a4ad6f382e75c20392b668cad9408bde9f42e
> --apihost=http://localhost:5000 test
>
> (where apikey can be obtained from the web UI, after logging in, going
> to 'My account' via you user name top right, then API keys.)
>
> This call is supposed to finally end up in the 'test' method in
> kallithea/controllers/api/api.py.
> However, the command fails with 500 Server Error, and on server side:
>
> Traceback (most recent call last):
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/wsgiapp.py",
> line 120, in __call__
> response = self.wrapped_dispatch(controller, environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/appwrappers/errorpage.py",
> line 56, in __call__
> resp = self.next_handler(controller, environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/appwrappers/caching.py",
> line 54, in __call__
> return self.next_handler(controller, environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/appwrappers/session.py",
> line 71, in __call__
> response = self.next_handler(controller, environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/appwrappers/i18n.py",
> line 71, in __call__
> return self.next_handler(controller, environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/wsgiapp.py",
> line 285, in _dispatch
> return controller(environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/kallithea-tg-clean/kallithea/lib/base.py",
> line 461, in __call__
> return super(BaseController, self).__call__(environ, context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/controllers/dispatcher.py",
> line 119, in __call__
> response = self._perform_call(context)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/controllers/dispatcher.py",
> line 99, in _perform_call
> state = self._get_dispatchable(context, py_request.quoted_path_info)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tg/controllers/dispatcher.py",
> line 73, in _get_dispatchable
> state = state.resolve()
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/crank/dispatchstate.py",
> line 178, in resolve
> return self._root_dispatcher._dispatch(self, self._path)
>   File 
> "/home/tdescham/repo/contrib/kallithea/venv/kallithea-tg-clean/lib/python2.7/site-packages/tgext/routes/dispatch.py",
> line 146, in _dispatch
> action = getattr(controller, action_name)
> AttributeError: 'ApiController' object has no attribute 'index'
>
>
> So the routing tries to access an index method.
>
> In the API code, the JSONRPCController parent class of ApiController
> is supposed to direct the calls to the correct method, but it seems we
> do not enter that class.
>
> Alessandro, would you have some time to have a look?
>
> I have seen in the TG2 documentation that typical, recommended, usage
> is different than what Kallithea is doing, and I think we should
> investigate if we can adapt our code base to such best practices
> eventually, but in the short term I think we should try to make the
> existing code base work.

This is a gentle reminder in case you missed this mail,

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