root out SQL building

[chadwhitacre]

There's a couple places where we build SQL through concatenation. We're starting to look at porting to SQLAlchemy (#129), which, if carried to completion, would take care of all this. In the mean time we should get rid of the couple places where we construct SQL manually. If I'm not mistaken this is just a couple where clauses.

[chadwhitacre]

@cyberjacob @artagnon @JustinLilly @lyndsysimon @alexcouper Anyone want to tackle this one?

[alexcouper]

I wouldn't have thought this would be doable before gittip has ported to SQLAlchemy - or did "this one" encapsulate changing over to SQLAlchemy as well?

[lyndsysimon]

@alexcouper I think it would best to go ahead and move to SQLAlchemy on a chunk-by-chunk basis, per comments in #129.

I don't think I'll be able to tackle this, as I've not used SQLAlchemy yet without the declarative syntax. I will be watching the implementation though, and will probably throw in an opinion or two and may join in little bit later :)

[alexcouper]

@lyndsysimon I agree - I guess I'm a little puzzled as to where #405 lives in relation to the end of #129.

@mjallday 's last two comments seemed to suggest that
a) he was going to begin the move over in a separate branch and
b) he'd be starting by migrating authentication.py

I'm unsure whether there's room for more than one person to do the first step - as someone will want to define centrally the models/table structure - but I could be wrong.

Or has (a) already happened?

[chadwhitacre]

Blech. Okay! Found it: 9168a2e. The release script assumes that it can safely modify and push gittip/init.py. That wasn't true in this case. Underlying issue ticketed as #410.

[lyndsysimon]

Okay, now I'm really confused. I'm just going to sit back and watch for a while :)

[chadwhitacre]

@alexcouper @lyndsysimon Back to the point at hand, I expect the ORM work to take some doing, and de-replacing SQL might be easier. I've fixed the instance that @mjallday flagged on #129. The other two places we do this are:

https://github.com/whit537/www.gittip.com/blob/3933fabc6fef7fc953cd7fc1cbf3eac9735506b3/gittip/__init__.py#L305

and

https://github.com/whit537/www.gittip.com/blob/3933fabc6fef7fc953cd7fc1cbf3eac9735506b3/gittip/billing/__init__.py#L58 (three instances; search for '%%')

In both it's clearer what's going on, I think. In the authenticate.py case the string replacement was coming in as an argument to the function, so even though it was only called just above it felt a lot worse. In the other two cases there's both a lot more overlap and a lot less being changed.

[chadwhitacre]

Mostly though I'm just looking for good ways to involve you all in contributing. :-)

If there are other tickets that catch your interest then let's talk about 'em! :-)

[chadwhitacre]

I believe stored procedures would be a way to solve this.

[chadwhitacre]

We also do this a few places in test fixture.

[alexcouper]

I'm interested in contributing more - possibly on this ticket, or another if this is done before I have the opportunity.

The SQLAlchemy porting is more of interest to me than changing the way SQL strings are formed TBH.

I'm tied up for the whole of December unfortunately, but I'm pledging 4 full working days in January to gittip. (There should be a site for that kind of OSS pledge). I'll get in touch on the dates I'll be doing that.

[chadwhitacre]

@alexcouper Sweet! :D

I'm going to close this ticket since we're moving forward with #129. Do you use IRC? The #gittip channel on Freenode is a great way to discuss in real-time.

[chadwhitacre]

@alexcouper I just added you as a collaborator on this repo. Welcome aboard. :-)

[alexcouper]

Thanks @whit537! I'm away at the moment but when I get back I'll be joining that IRC channel. FYI the 4 days in Jan look like they'll be spread between 15th and 26th, but I'll confirm closer to the dates.