tamentis / cartman

Command line Trac tools
http://tamentis.com/projects/cartman/
ISC License
22 stars 10 forks source link

Only login when needed #10

Closed strk closed 9 years ago

strk commented 9 years ago

Greately speeds up commands like "help"

strk commented 9 years ago

It would be also useful to save session cookies, to avoid more than a single GET on every single command

strk commented 9 years ago

@tamentis don't you like this approach ? I've started using cartman with trac.osgeo.org and I'm often slowed down by these upfront requests for no reason...

tamentis commented 9 years ago

I don't like adding a login() call in get(). Ideally I would prefer to do two things:

  1. like you said, save session cookies
  2. flag which commands require login (maybe a decorator)
strk commented 9 years ago

Some commands would still benefit from controlling when login happens. See the 'comment' command for example: immediately starting the editor and postponing login is a better experience

tamentis commented 9 years ago

Agreed, then for the purpose of that PR, I would rather see self.login() in the right places for specific commands than a parameter to .get()

strk commented 9 years ago

My assumption was that "the right place" is really whenever a "get" or a "post" are needed.

It can be some "get" which do not need authentication (like the one to send login) thus the added parameter.

It's much less intrusive that way.

tamentis commented 9 years ago

I'd like to keep get() as a wrapper around the HTTP GET method which implies a single query. I also don't like inverse flags like nologin and even though calling self.login() is more intrusive, it is a more explicit representation of what is actually going on to make that command work.

strk commented 9 years ago

inverse flag is gone, the other arguments still hold :)

strk commented 9 years ago

Alright, now changed to explicit call. See if you like it now.

tamentis commented 9 years ago

Awesome, thank you :)