/srv/irclogs.ubuntu.com/2010/02/01/#launchpad-reviews.txt

mwhudsonhm04:42
mwhudsoni don't suppose anyone is here?\04:42
* henninge is OCR but cannot change the channel subject anymore. Wonder why ...10:01
intellectronicahenninge: likewise :-/10:13
henningeintellectronica: they may have fiddled with the settings last week during UDW.10:13
henninges/during/for/10:13
intellectronicamaybe10:13
intellectronicahenninge: sorry for not having started with reviewing yet. i'm going for a fun little visit at the dentist's soon, but will be reviewing when i'm back, around lunchtime11:00
henningeintellectronica: I see. I will have to go out in the afternoon, too ... ;-)11:01
intellectronicahenninge: cool, so at least we're doing shifts11:01
=== mrevell is now known as mrevell-lunch
bachenninge: did you pursue the channel topic problem at all?  we need to get that resolved.  i can't change the topic in #launchpad either13:27
henningebac: no, I didn't. I don't have any clue were to start.13:28
bacme neither13:28
henninges/were/where/13:28
baci guess i'll try in #freenode13:28
henningebac: I was able to change to topic on #launchapad-dev, btw.13:29
bacme too13:30
baci'm seeing if our crack #is staff can help13:34
=== mrevell-lunch is now known as mrevell
adeuringintellectronica: do you know why we projoin the tables Product and SourcePackageName in BugtaskSet.search()?13:57
adeuringintellectronica: soory, wrong channel...13:57
bachenninge: can you have a look at https://code.launchpad.net/~bac/launchpad/bug-515171/+merge/18385 ? it is super trivial13:57
henningebac: r=me ;)14:01
bachenninge: thanks14:04
=== salgado is now known as salgado-lunch
jamaltaEdwinGrubbs: ping15:15
EdwinGrubbsjamalta: hi15:16
jamaltaEdwinGrubbs: hi, i had noted in my calendar to bug you today about landing an MP.. do you have some time?15:18
jamaltahere's the link https://code.edge.launchpad.net/~jamalta/launchpad/relatedbugs-118609/+merge/1814915:18
EdwinGrubbsjamalta: I'll try to land it. I'll let you know when it is in pqm.15:19
jamaltaEdwinGrubbs: thanks15:19
=== matsubara is now known as matsubara-lunch
intellectronica1 2 3 test15:49
Ursinhaintellectronica: tested15:50
intellectronicaUrsinha: thanks :)15:50
Ursinhaintellectronica: :)15:50
andrea-bsHello everybody. I've just pushed a very small branch that's ready for review: https://code.edge.launchpad.net/~andrea-bs/launchpad/fix-411007/+merge/1838915:51
abentleyandrea-bs: approved.  Once it has passed the test suite, I will be happy to land it for you.16:03
andrea-bsabentley: thanks!16:03
andrea-bsabentley: oops! I've just noted that the branch has conflicts16:05
abentleyandrea-bs, I don't see any conflicts in the text.  I think the web page is bugged.16:06
andrea-bsabentley: yes, there's a bug in LP. I'm going to report it16:06
=== salgado-lunch is now known as salgado
Rinchenwell bac, in order to fix this I need to be an op here16:09
Rinchenso I'll need to get help for that16:09
jpdsRinchen: The channel isn't registered to begin with...16:09
bacRinchen: yeah, what jpds said16:09
jpdsRinchen: Hi! too.16:09
Rinchenjpds: yeah wasn't on the list when I was with LP to setup16:09
jpdsRinchen: Poke someone on: /who freenode/staff/*16:09
bacRinchen: i think people just create some channels without knowing there is a procedure to register them16:10
* bac didn't know...16:10
UrsinhaRinchen: is the channel registered?16:10
Rinchenno Ursinha 16:10
UrsinhaRinchen: if not, you can register and you're the op16:10
Ursinhathen set guard mode on and give op to others16:10
maxbYou can only register a channel whilst you *are* an op of it16:11
Ursinhamaxb: you're right, if the channel has people on it...16:11
Ursinhathat was easy :)16:12
* Rinchen has connections16:12
maxb#launchpad-foundations is in need of the same treatment16:13
Rinchenbac: making progress but getting an error from chanserv setting privs16:20
abentleyRinchen, I am an OCR today, so I'll want to be able to change the topic, whether that requires ops or whatever.16:22
Rinchenabentley, bac - try oping yourselves please16:22
jpdsRinchen: /mode -t16:22
abentleyRinchen, it doesn't work yet.16:22
* bac power!16:22
Rinchensuper thanks bac16:23
bacRinchen: you did the hard work16:23
RinchenI need to finish a comparison against #launchpad but it appears everyone should have the same access both places16:23
bacRinchen: did you change the mode to -t ?16:23
bacintellectronica: could you try changing the topic here?16:24
RinchenI had an error in the channel flags16:24
Rinchenyou'll need to reop bac16:24
intellectronicabac: i am told that i am not channel operator16:24
abentleySame here.16:24
jpdsintellectronica: I think you need to identify to NickServ.16:25
* abentley is already identified.16:25
bacintellectronica: please try now16:25
bacto change topic16:25
intellectronicai am still not channel operator16:26
Rinchenintellectronica: the hash is set to those with @canonical or @freenode nicks16:26
jpdsabentley: But no c/lp cloak. :-/16:26
bacoh, i see chanserv put it back to +t right after i did -t16:26
abentleyjpds, been on freenode years before I was with Canonical.16:26
bacdamn16:27
abentleybac, maybe if you ask ChanServ to set the mode?16:27
bacabentley: how?16:27
EdwinGrubbsjamalta: I'm running the full suite of tests even though it is unlikely that anything broke with that small change. You can see the progress here: http://ec2-174-129-125-7.compute-1.amazonaws.com/16:27
abentleybac, I think the flags command would do it.16:28
bacabentley: even though i'm an op it says i am unauthorized16:28
jpdsRinchen: /msg ChanServ set #launchpad-reviews mlock +nc-tslk16:28
Rinchenjpds: already done16:29
Rinchenjpds: as in, I removed it16:29
jpdsNow -t should be removable.16:29
=== matsubara-lunch is now known as matsubara
jamaltaEdwinGrubbs: thanks for the update, will try and take a look at the link16:30
=== jamalta is now known as jamalta-afk
=== jamalta-afk is now known as jamalta
al-maisananybody up for a review?16:46
al-maisanhttps://code.edge.launchpad.net/~al-maisan/launchpad/ejdt-n-1/+merge/1839116:46
bacjtv: ping16:59
jtvbac: hi16:59
jtval-maisan: your mp reports conflicts17:15
al-maisanI can merge devel trunk w/o conflicts though17:16
al-maisan.. and the other way around: I can merge the branch under review into a fresh devel branch w/o conflicts17:17
abentleyjtv, if there were actually conflicts, the affected files would be listed immediately below.17:20
jtvabentley: then what does it mean for the mp to say that there are conflicts?17:20
al-maisanyeah .. it's quite confusing17:20
abentleyjtv, it means there's a bug, and that the web page treats anything that's not None as conflicts, including ''.17:21
jtvabentley: ah, known bug?17:21
abentleyjtv, yes.17:22
jtvabentley: thanks for explaining.17:22
jtval-maisan: minor æsthetic note: in get_builder_data, ll. 50—73, consider moving the computation of totals outside the loop...  sum([count for ((processor, virtualized), count) in builder_stats.iteritems() if virtualized == True])17:34
jtvAlthough if that's making things too complicated, nm.17:34
al-maisanjtv: let me take a look17:34
jtvgood job catching what looked like an accidental interaction between "processor" and "virtualized" in the SQL composition code btw.17:36
al-maisanjtv: thanks :)17:36
jtval-maisan: on another note, there is a way to stormify non-object queries ("SELECT count(*) FROM foo" and such)  though I don't recall the syntax off the top of my head.17:39
al-maisanhmm .. hmm..17:40
jtvBad example; I meant things like "SELECT bar, count(*) FROM foo ORDER BY bar"17:40
al-maisan"GROUP BY"?17:41
jtvSorry, yes.17:41
jtval-maisan: for ll. 298—299, the head_job_platform query, can't you just write "return store.execute(query).first()"?17:46
al-maisanjtv: I guess so, that should be better.17:47
jtvoh, dinner gong17:47
=== jtv is now known as jtv-eat
al-maisanjtv-eat: enjoy your dinner :)17:48
al-maisans/first/get_one/17:52
al-maisanjtv-eat: http://pastebin.ubuntu.com/366981/17:55
* al-maisan is being called to dinner as well17:56
=== EdwinGrubbs is now known as Edwin-lunch
=== jtv-eat is now known as jtv
jtval-maisan: in line 409, getEstimatedJobStartTime reads builder_stats[platform], where builder_stats comes out of get_builder_data.  Is it absolutely certain that that entry exists in the dict?19:14
al-maisanjtv, just a second19:14
al-maisanjtv: yes19:14
al-maisanjtv: ..and if not a 0 will be returned19:15
al-maisansince I am using a defaultdict foir builder statistics19:15
jtvoic19:15
al-maisan*for19:15
jtval-maisan: I wonder if maybe the normalization of head_job_platform should move into _getHeadJobPlatform.19:18
al-maisanjtv: I prefer normal functions if I can .. also the normalize_virtualization() function is used in quite a few places unrelated to the head job..19:21
jtval-maisan: but maybe this particular invocation would look better inside _getHeadJobPlatform()?19:22
jtvSo imagine the whole "if <result of _getHeadJobPlatform()> is None" part would move into _getHeadJobPlatform.19:25
* al-maisan looks at the code again19:25
bacabentley: would you do an experiment and see if you can change the topic here?19:26
* bac still wresting freenode19:26
=== abentley changed the topic of #launchpad-reviews to: on-call: abentley || reviewing: || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
abentleybac, yay!19:27
bacabentley: cool, thanks19:27
al-maisanjtv: are you suggesting to move lines 182-185 into _getHeadJobPlatform()?19:27
jtval-maisan: 181—185 even19:28
jtvAnd then you could probably do without line 204 and rewrite line 189 slightly19:28
al-maisanjtv: sorry .. that's what I meant to ask .. :P19:28
jtv:)19:28
al-maisanjtv: yeah .. I can make that change if desired.19:35
jtval-maisan: thanks, my impression is it'll make for a cleaner composition.19:36
al-maisanno problem19:40
al-maisanjtv: I agree19:41
jamaltaEdwin-lunch: thanks so much, looks like pqm finished :)19:48
jtval-maisan: next point...  _freeBuildersCount sounds like you're freeing a builders count!19:52
al-maisanjtv: hmm .. I guess you're right .. it does sound like an action19:52
al-maisanjtv: how about _countFreeBuilders() ?19:53
jtvsure19:54
al-maisan.. or _getFreeBuildersCount()19:54
jtvfine19:54
al-maisanthe latter is probably more conventional19:54
jtvyes, and avoids the notion that the counting itself is an action that you want the method to undertake.19:55
jtvIt's the result that we're interested in.19:55
=== Edwin-lunch is now known as EdwinGrubbs
al-maisanhmm .. let's settle for _getFreeBuildersCount() then19:55
jtvAgreed.19:55
jtval-maisan: the string on ll. 239—255 is doubly indented and the closing quotes are not indented far enough.20:01
al-maisanjtv: good catch .. I'll fix that.20:02
=== matsubara is now known as matsubara-afk
jtval-maisan: in _getPendingJobsClauses, another way to express "buildqueue.virtualized = %(virtualized)s OR (buildqueue.virtualized IS NULL AND %(virtualized)s = TRUE" would be "COALESCE(buildqueue.virtualized, TRUE) = %(virtualized)s"20:06
jtvCOALESCE returns the first of its arguments that is non-null20:06
al-maisanjtv: very cool20:07
jtvOR clauses can make things so ugly...  :)20:07
al-maisanjtv: COALESCE is definitely more elegant :)20:09
jtval-maisan: and another tip, more as background information: when composing SQL clauses like this, we often get more maintainable code by constructing a list of conditions and joining them with " AND ".join(conditions)20:10
jtvIn this case it wouldn't matter much though.20:10
al-maisanjtv: yeah .. I agree, but in this case the clauses were all identical .. I was just looking to avoid repetitive code.20:11
jtvRight ho.20:12
=== salgado is now known as salgado-afk
jtvBTW the pattern of queries here suggests that you may be interested in the "DISTINCT ON" feature as well.  It lets you say things like "give me only one buildqueue/job row per (lastscore, id) value, and ignore any rows with the same values that come later in the query's ordering."20:13
jtv(Again just background info, not trying to coerce you into using it here)20:14
al-maisanhmm .. I see.20:14
jtval-maisan: in the docstring for getEstimatedJobStartTime, you mention "(weighted by the number of machines in the respective build pool)."  Is the "weighted by" really the right term here?  Not "divided by" or "distributed across" or somesuch?20:17
al-maisan"divided by" is probably better20:17
jtvthanks, that clarifies things for me20:17
jtval-maisan: unnecessary parentheses on line 440 of the diff, in test_buildqueue.py.20:19
al-maisanjtv: noted, thanks!20:19
jtvThat's an import.  Or alternatively, the line needs to be broken up.  :-)20:19
al-maisanremoving parentheses is probably better20:20
jtvYes, the choice depends entirely on line length20:24
jtval-maisan: for assertEqual tests, we've standardized on passing the expected value first and the actual value second.  Not asking you to chance all the existing ones, but maybe the ones you touched?20:26
al-maisanjtv: no problem -- I'll probably do all in one fell swoop20:27
jtvcool!20:27
jtval-maisan: in test_buildqueue, you're disabling all native builders... is there no way to do that in a single loop without exhaustively naming architectures?20:29
al-maisanjtv: yeah .. should be doable .. will fix that.20:29
jtvthanks, I think i'll eliminate a distraction from the test.20:30
al-maisanjtv: I will be signing off now .. if there's anything else that needs doing please mention it in the review email.20:44
jtval-maisan: sure, no worries20:44
jtvI'll try to live up to my fascist reputation  :-P20:44
al-maisanjtv: ah, c'mon, admit that you like that ;)20:45
jtval-maisan: oh YES I do!20:45
jtvlinks-zwo-drei-vier...20:45
al-maisanjtv: now it's on record :-)20:45
al-maisanjtv: thank you very much again for reviewing my branch and good night :)20:46
jtval-maisan: well, at least nobody will accuse me of being *crypto*-fascist.  :-)20:46
al-maisanoh, that's an advanced rank ;)20:46
* al-maisan falls of the chair20:47
=== abentley changed the topic of #launchpad-reviews to: on-call: - || reviewing: || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews
=== jamalta is now known as jamalta-afk

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!