Review: Approve code 78 +def get_archive_privacy_filter(user): 79 + return [ 80 + Not(Archive._private), 81 + Archive.ownerID.is_in( 82 + Select( 83 + TeamParticipation.teamID, 84 + where=(TeamParticipation.person == user)))]
The indentation here is wrong, and it might be better to just say Or() rather than returning a list. 174 + clauses.extend( 175 + [BinaryPackageBuild.package_build == PackageBuild.id, 176 + PackageBuild.build_farm_job == BuildFarmJob.id]) The opening square bracket should probably be on the previous line. 221 - AND SourcepackageName.name LIKE '%%%%' || %s || '%%%%' 222 - ''' % quote_like(name)) 223 + clauses.append(SourcePackageName.name.like(name)) These aren't equivalent. You need to use quote_like and wrap it in % to get a substring match; see Distribution.searchSourcePackageCaches's LIKE stuff for an example of hacking around Storm. There's also another instance of this in the following block. 274 + if user is None: 275 + clauses.append(Not(Archive._private)) 276 + elif not IPersonRoles(user).in_admin: 277 + clauses.append(*get_archive_privacy_filter(user)) Shouldn't this be in get_archive_privacy_filter? 279 + clauses.append(BuildFarmJob.builder_id == builder_id) Can you put this in the initial clauses? 288 - queries = [] 289 - clauseTables = [] 290 + clauses = [] 291 + origin = [PackageBuild] Why the new table? -- https://code.launchpad.net/~stevenk/launchpad/stormier-getbuildsforbuilder/+merge/126159 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
