Code review comment for lp:~jcsackett/launchpad/convert-sql-627631-storm

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi JC,

It's a good thing Benji pointed out the odd leading underscore in "_translations_usage" since this is the cause of a serious bug in your queries, and it is probably something that storm or the sqlobject layer on top of it should warn about.

Product.selectBy(translations_usage=ServiceUsage.LAUNCHPAD) actually generates the SQL:
 WHERE FALSE
since Person.translations_usage is a property object and not a storm attribute.

Product.selectBy(_translations_usage=ServiceUsage.LAUNCHPAD) generates the SQL:
 WHERE Product.translations_usage = 20

The need for a property to override the setter can be done in storm with a validator. However, I'm not sure if you will still need to override the getter, which would still require a translations_usage property, and the storm attribute would still need to be renamed _translations_usage.

-Edwin

« Back to merge proposal