=== jamesh_ is now known as jamesh | ||
=== tasdomas_afk is now known as tasdomas | ||
=== Ursinha-afk is now known as Ursinha | ||
=== Ursinha is now known as Ursinha-afk | ||
=== Ursinha-afk is now known as Ursinha | ||
=== tasdomas is now known as tasdomas_afk | ||
cjwatson | StevenK: sorry, it's in archivepublisher, not archiveuploader, because custom uploads | 15:06 |
---|---|---|
cjwatson | StevenK: it appears to actually be in each CU implementation | 15:06 |
StevenK | cjwatson: Yes, I just determined that, but thanks. | 15:07 |
cjwatson | wgrant: The semantics of BuilderStatus.ABORTED on the master side seem a bit different from what we want from BuildStatus.ABORTED. BuilderStatus.ABORTED ends up in BuildFarmJobOld.jobAborted, which sets the status to BuildStatus.NEEDSBUILD; I think we want BuildStatus.ABORTED to end up in BuildStatus.CANCELLED. | 15:23 |
cjwatson | But maybe that means that (a) we want it to be called CANCELLED on the slave side, (b) perhaps this isn't quite so compatible with the rescue case after all? | 15:23 |
wgrant | cjwatson: It depends on the origin state | 15:23 |
wgrant | BUILDING + ABORTED -> NEEDSBUILD | 15:24 |
wgrant | CANCELLING + ABORTED -> CANCELLED | 15:24 |
cjwatson | IOW it should depend on that, not depends on that right now? | 15:24 |
cjwatson | (Since I don't see that in the code) | 15:24 |
wgrant | Yeah | 15:24 |
wgrant | That's how it probably should work | 15:25 |
cjwatson | Mkay | 15:25 |
wgrant | The slave doesn't need to know why it's being cancelled | 15:25 |
wgrant | That's purely a master-side policy decision | 15:25 |
wgrant | The slave may be being rescued, or it may be being cancelled. | 15:25 |
wgrant | They are identical from the slave POV. | 15:26 |
cjwatson | OK. Bit confusing for BuildStatus.ABORTED not to necessarily be jobAborted, but I suppose there are only so many verbs. | 15:26 |
wgrant | Well | 15:27 |
wgrant | jobAborted is never called atm. | 15:27 |
wgrant | Because ABORTED is only ever hit after a rescue | 15:27 |
wgrant | In which case the job is no longer assigned to the builder, so there's no job to call jobAborted on. | 15:27 |
wgrant | An ABORTED slave for a BUILDING build should probably be an OOPS | 15:28 |
wgrant | Because I can't see how that could occur | 15:28 |
cjwatson | It's quite possible it is an OOPS right now, then | 15:29 |
wgrant | jobAborted might not even make sense. | 15:31 |
wgrant | The job is cancelled | 15:31 |
wgrant | handleStatus_ABORTED might call jobCancelled when status was CANCELLING, and either set it back to NEEDSBUILD or OOPS and set to FAILEDTOBUILD for any other status | 15:32 |
cjwatson | wgrant: https://code.launchpad.net/~cjwatson/launchpad/buildstatus-aborted/+merge/176990 | 16:31 |
wgrant | cjwatson: I think that looks fairly sane | 16:39 |
wgrant | I'll have to think about it | 16:40 |
wgrant | Well | 16:40 |
wgrant | And try it | 16:40 |
wgrant | Until it breaks | 16:40 |
StevenK | cjwatson, wgrant: https://code.launchpad.net/~stevenk/launchpad/no-dead-custom-uploads/+merge/176995 | 16:52 |
cjwatson | StevenK: the setComponents description in customupload.py should probably be more generic | 16:56 |
cjwatson | StevenK: and don't you want target_series.enabled_architectures instead of .architectures? | 16:57 |
StevenK | - """Set self.version and self.arch (if applicable).""" | 16:57 |
StevenK | + """Set instance variables based on decomposing the filename.""" | 16:57 |
cjwatson | Seems fair | 16:58 |
StevenK | cjwatson: That's a question, do we want to copy custom uploads for arches which are disabled? | 16:58 |
cjwatson | I don't think so | 16:58 |
StevenK | That's more a question for you, or infinity | 16:58 |
cjwatson | That was the raring(?)/armel case | 16:58 |
StevenK | Right. This also gives us grace to disable them from the get go, and then yay, no custom uploads on first publish | 16:59 |
cjwatson | Once it's disabled, I think we want to pretend it doesn't exist, it's just that we can't delete it easily | 16:59 |
wgrant | Well, the raring/armel case would still have worked | 17:02 |
wgrant | Because this is for the target arch check | 17:02 |
wgrant | But yes | 17:02 |
wgrant | enabled_architectures should be the default | 17:02 |
cjwatson | https://code.launchpad.net/~cjwatson/launchpad-buildd/fix-abort/+merge/177003 *phew* | 17:34 |
=== Ursinha-afk is now known as Ursinha | ||
=== Ursinha is now known as Ursinha-afk | ||
=== Ursinha-afk is now known as Ursinha | ||
cjwatson | wgrant: Is there a plausible way to get a callback in buildd-manager when a build goes to ABORTED (for the bit during cancellation that needs to wait for that), or do I just have to poll? | 21:56 |
wgrant | cjwatson: You have to poll. I'd suggest using the separate handleStatus_ABORTED and just working off the state | 21:57 |
wgrant | Rather than doing it as part of the cancellation function | 21:57 |
cjwatson | Hm, yeah, that makes sense. | 21:57 |
wgrant | Unless you want to rewrite buildd-manager's state machine | 21:58 |
cjwatson | No :-) | 21:58 |
wgrant | Which somehow seems unlikely.. | 21:58 |
cjwatson | Still need a timeout which I cancel, but I guess I can manage that | 21:58 |
wgrant | Yeah, I'm not quite sure how to do the timeout. | 21:58 |
wgrant | Possibly store is as a local attribute in buildd-manager | 21:59 |
wgrant | It means that a restart will reset the timeout, but meh | 21:59 |
wgrant | Really doesn't matter | 21:59 |
wgrant | And is probably cleaner | 21:59 |
cjwatson | Why not just reactor.callLater? | 21:59 |
wgrant | A good point. | 21:59 |
wgrant | Well | 21:59 |
wgrant | A restart would stop that from working | 21:59 |
cjwatson | Though I still need to store the Deferred somewhere | 22:00 |
wgrant | eg. I start cancelling a build, then buildd-manager restarts | 22:00 |
wgrant | If you just naively use callLater then it will never time out | 22:00 |
wgrant | So, I guess, ideally it would see slavestatus == BUILDING and masterstatus == CANCELLING, and call abort() and set a timeout | 22:01 |
wgrant | If it seeis slavestatus == ABORTING and masterstatus == CANCELLING, it might have to ensure that a timeout is set. | 22:02 |
cjwatson | Maybe the simplest answer is just to check each time SlaveScanner notices a masterstatus == CANCELLING build, since it's polling every 15 seconds anyway | 22:05 |
cjwatson | It could easily just keep count of a multiple of that | 22:05 |
cjwatson | Not very Twisted, but | 22:05 |
wgrant | Yeah, indeed | 22:07 |
cjwatson | If we were really trying hard then we could add a DB column on Builder for the timeout | 22:08 |
wgrant | That would be trying altogether too hard. | 22:13 |
=== _mup__ is now known as _mup_ |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!