Page MenuHomeSoftware Heritage
Paste P558

about where/when to put the lister's needed scheduler task types: conclusion in the constructor
ActivePublic

Authored by ardumont on Nov 8 2019, 12:00 PM.
11:18 <+ardumont> douardda: D2237 draft to check for missing task types
11:18 -- Notice(swhbot): D2237 (author: ardumont, Needs Review) on swh-lister: lister: Add checks on expected scheduler's output tasks <https://forge.softwareheritage.org/D2237>
11:18 <+ardumont> i'm not sure when/where to plug that check though
11:21 <+olasd> if we're doing that, we might just as well create the task type with the proper settings
11:25 <+ardumont> mmm, unsure
11:25 <+ardumont> my take is that it's more the loader's job to do it (to match what the listers are doing and some loader; deposit comes to mind)
11:26 <+olasd> the deposit issue was the lister creating tasks with the right type but the wrong signature; nothing in our system currently prevents that
11:26 <+ardumont> i'm not talking about that issue
11:26 <+olasd> or, well, the "thing that creates tasks"
11:26 <+ardumont> deposit uses the same registering mechanism as the lister
11:27 <+ardumont> which includes the task types creation
11:27 <+ardumont> about the listing
11:27 <+ardumont> so to match that, i think other loaders should align
11:28 <+olasd> ah, I see
11:28 <+ardumont> (well for deposit, it's not listing, it's loading but you grok the idea ;)
11:29 <+ardumont> in any case, i still think we need that check
11:29 <+ardumont> to make the lister failing as early as possible
11:29 <+ardumont> lister fail*
11:29 <+olasd> sure
11:29 <+ardumont> i'm not a huge fan of side-effect within the constructor
11:29 <+ardumont> but it seems the right place
11:30 <+ardumont> well at least, it seems reasonable to put it there
11:30 <+olasd> it's not a side effect, it's a configuration check
11:30 <+ardumont> side-effect as in implementation wise
11:30 <+olasd> a side effect would be creating the task type
11:30 <+olasd> :P
11:31 <+olasd> there's no point initializing if you'll be unable to do your work
11:31 <+ardumont> yes
11:31 <+ardumont> you are starting to convince me ;)
11:31 <+ardumont> good
11:31 <+ardumont> i'll amend the diff
11:32 <+ardumont> btw, you guessed it, i had the issue with the debian lister on the staging infra
11:32 <+ardumont> somehow the type there is load-debian-package instead of load-deb-package