Code review comment for ~ilasc/launchpad:git-repack-cron-script

Revision history for this message
Ioana Lasc (ilasc) wrote :

Thanks Colin, excellent suggestions as usual!

This is now working with a TunableLoop but needs more work: the TunableLoop doesn't hook into the fake turnip thread for the calls to gitHostingClient and it is oops-ing with: https://pastebin.canonical.com/p/6RKM3kfkTT/
I'm working on getting the LaunchpadCronScript and TunableLoop to interact with Fake Turnip Thread in unit test instead of trying to contact the real application.

In terms of the permissions added to the branchscanner user, this is how it crashed if SELECT rights are not granted to the user:https://pastebin.canonical.com/p/qVwRKjztzM/
And it appears I've gone overboard with permissions as UPDATE is not necessary, so will be taking that out.

I didn't miss your last comment about adding at least another test for the case where we have more than single repository qualifying for repack. As soon as I get the Unit Tests to play nicely with the FakeTurnipApplication will be adding that in too.

« Back to merge proposal