[08:52] <cjwatson> tomwardill: Could you have a look over https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/380350 and https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/380351 for OCI webhooks?
[08:52] <tomwardill> oh, I meant to do that yesterday, had the tabs open and everything
[08:52] <tomwardill> on it
[08:52] <cjwatson> ilasc: Could you have a look at https://code.launchpad.net/~cjwatson/lp-signing/+git/lp-signing/+merge/380014 ?  It's a prerequisite for the Juju charm
[08:53] <cjwatson> Also, yesterday I found https://forge.softwareheritage.org/T1734 which relates to my recent GitRepositorySet.getRepositories changes, and updated it with a slew of details
[08:56] <ilasc> cjwatson: +1
[08:59] <ilasc> have mp 380391 open for study on the to do list for today, above detail is great for that! thanks for sharing Colin!
[08:59] <wgrant> Ah, excellent, that hopefully solves much of the problem.
[09:00] <cjwatson> wgrant: Which does?
[09:00] <wgrant> That softwareheritage.org ticket, and your reply to it.
[09:00] <cjwatson> Ah yes
[09:01] <cjwatson> There's a SH bod on #debian-uk who said that the people working on this are students who just kind of ... show up.  Must be a nice problem to have :)
[09:01] <tomwardill> can we borrow some :)
[09:03] <ilasc> :)
[09:07] <wgrant> That does make some degree of sense.
[09:08] <wgrant> And would indeed be quite handy.
[09:08] <wgrant> Though I guess a bunch of us were just students who showed up at one point.
[10:37] <cjwatson> Hmm I think I can possibly reproduce the turnip-pack-virt memory leak locally
[10:40] <cjwatson> Doesn't seem to reproduce when looking up nonexistent repos, but if I clone the same existent repo 100x in a loop then I see virtserver's VIRT growing by a MB or so in top.  (sshserver gets bigger too, but I think that's keepalive connections and goes away again)
[10:40] <cjwatson> Attacking it with meliae now to see if I'm imagining it
[10:40] <tomwardill> ... I've made the wifi slightly faster in my study by replacing the cable
[10:40] <tomwardill> one of us is having a productive morning. I suspect it's not me.
[10:43] <cjwatson> you're optimising your environment! :)
[10:50] <cjwatson> suspicious growth in function objects
[10:54] <cjwatson> all in the guts of inlineCallbacks I think
[10:57] <SpecialK|Canon> Investing in your infrastructure is generally worthwhile!
[11:00] <cjwatson> I don't think PackVirtServerProtocol.requestReceived ever returns in the successful case
[11:00] <cjwatson> I stuck a log entry at the end that I'm not seeing
[11:04] <cjwatson> Oh.  Well nothing ever calls .callback() on that particular deferred, only .errback().  That would probably do it
[11:05] <cjwatson> (see PackClientFactory)
[11:07] <cjwatson> is it a one-liner
[11:08] <SpecialK|Canon> hah
[11:12] <cjwatson> Need to figure out how to test it but I'm pretty sure it's just https://paste.ubuntu.com/p/vRJsZtMhcW/
[11:12] <cjwatson> meliae++, made this a great deal easier to track down
[11:27] <tomwardill> cjwatson: is it worth adding the date_* columns to the OCI credentials and push rules?
[11:35] <cjwatson> tomwardill: I don't think so really.  They're not going to be displayed on their own pages with the "Registered by <foo> on <bar>" boilerplate or anything; they're more like settings.
[14:50] <tomwardill> cjwatson: the credentials field, what do I need to do to make it an encrypted field?
[14:58] <cjwatson> tomwardill: There isn't yet any prior art for making an entire column be encrypted (as opposed to one value in a larger JSON column).  But you should be able to work it out, more or less, by grepping for EncryptedContainer
[14:58] <tomwardill> ah, that's the magic term i was missing
[14:58]  * tomwardill tries
[14:59] <cjwatson> tomwardill: You'll need an IEncryptedContainer implementer (based on NaClEncryptedContainerBase) registered with some appropriate name and that plugs in the correct configured key pair; and you'll need a property on the model that does getUtility(IEncryptedContainer, whatevernameyoupick).{encrypt,decrypt}
[15:00] <cjwatson> tomwardill: Or if you wanted to try slightly harder you could write a custom Storm variable that does this
[15:00] <cjwatson> Shouldn't be very difficult, we just haven't needed it so far
[15:00] <tomwardill> the latter feels more resuable
[15:00] <cjwatson> Well, variable and property, since they usually come as a paid
[15:00] <cjwatson> *pair
[15:01] <cjwatson> e.g. have a look at the implementation of JSON and JSONVariable
[15:01] <cjwatson> And yes, I think that would be more correct
[15:27] <cjwatson> tomwardill: I think you can go ahead with https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/380200 now - the linked code change is on prod
[15:28] <tomwardill> ah, cool
[15:28] <tomwardill> landing
[16:08] <tomwardill> cjwatson: slightly trivial question, would the storm variable live in storm, or launchpad?
[16:19] <cjwatson> tomwardill: Launchpad
[17:12] <wgrant> I don't much like the Storm variable approach. It would make accidental leaks easier, and leave the object unloadable on most services
[17:13] <tomwardill> wgrant: I don't follow either objection there
[17:14] <wgrant> Accidentally printing the object's fields would leak the secret, and examining the object's fields on most LP hosts, that don't have the wrapping key, would crash.
[17:15] <tomwardill> aha, right, that makes sense
[17:17] <cjwatson> I suppose
[17:18]  * tomwardill closes storm checkout
[17:18]  * cjwatson finally persuades turnip tests to pass with only a small hack
[17:27] <tomwardill> excellent, DB patch landed with only minor buildbot poking
[17:28] <SpecialK|Canon> Nice
[17:33] <cjwatson> Could I have a review of https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/380646 please?  As far as I can tell this fixes the turnip-pack-virt memory leak on production that's been taking down the service every couple of days
[17:34] <SpecialK|Canon> Excellent!
[17:36] <tomwardill> looking
[17:39] <tomwardill> test_git.py is inconsistent as to assert(expected, received) or assert(received, expected)
[17:40] <cjwatson> Is it?
[17:41] <cjwatson> Oh, I got this backwards, silly me
[17:41] <tomwardill> test_git_receive_pack_calls_spawnProcess appears to be reversed from test_git_upload_pack_calls_spawnProcess
[17:41] <tomwardill> so there's both in there already
[17:41] <cjwatson> receive is using assertThat not assertEquals
[17:42] <cjwatson> and assertThat requires the matcher as second arg
[17:42] <tomwardill> oh, so it is
[17:42] <tomwardill> sorry
[17:42] <cjwatson> so it was just my new code that was wrong, and I've fixed it now
[17:42] <tomwardill> cool
[17:42] <tomwardill> cjwatson: what fails in the test without the callback change?
[17:43] <cjwatson> it hangs
[17:43] <cjwatson> since yield requestReceived never returned
[17:43] <tomwardill> ah... that'd do it :)
[17:43] <cjwatson> *returns
[17:43] <cjwatson> (though doesn't hang indefinitely since AsynchronousDeferredRunTest has a timeout)
[17:43] <tomwardill> have a +1
[17:44] <cjwatson> thanks