Skip to content

Check base class needs to incorporate ABC and parse time validation of sources #780

@ferringb

Description

@ferringb

There is a QoL feature for check writing that removed the need to manually specify the feed source; the feed source is what the check consumes. A single CPV, the CPVs of a package, the CPVS of a category, etc. That's a feed source.

See

@property
def source(self):
# filter pkg feeds as required
if self.known_results.intersection(self.options.filter):
if self.scope >= base.version_scope:
return (
sources.FilteredRepoSource,
(sources.LatestVersionsFilter,),
(("source", self._source),),
)
elif max(x.scope for x in self.known_results) >= base.version_scope:
return (
sources.FilteredPackageRepoSource,
(sources.LatestPkgsFilter,),
(("source", self._source),),
)
return self._source
source is consumed at runtime and used to know how to invoke the check; this means that an invalid _source, or results not defined- an incorrectly defined Check- fails at runtime.

ABC didn't exist when this got wrote, nor __init_subclass__, but they exist now, and this sort of thing needs to be moved to parse time.

In terms of ABC, every hasattr/gettattr in pkgcheck pipeline and feed subsystem needs to be checked for moving it into ABC. That's not trivial, thus I think we should reconsider this design.

The 'feed' as an attribute is probably the wrong design; the "figure out the feed automatically" is a nice QoL, but we also trade away the possibility of a properly typed def feed(self, ....) signature.

If the feed were a class inheritance- meaning we go back to the manual feed specification- that gets us type checking back. That's both good since our type alignment in pkgcheck isn't great (meaning: bugs), and since anyone writing a check has to go digging into pkgcore code and is writing a fair bit blind. Checks aren't simple things, especially the bash related ones, so making it easier via exposing the annotations is preferable since these days everyone uses IDEs, and this ought to make writing a check less of a PITA, including the edge cases.

^^^ that's not a small change, but I think it's worth doing, even if it's against my habitual norm of "use meta crap because I hate boilerplate".

What are people's thoughts on this?

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions