Re: [pve-devel] [PATCH access-control 1/1] add realmsync plugin for jobs and CRUD api for domainsync-jobs

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Nov 08, 2022 at 10:24:50AM +0100, Thomas Lamprecht wrote:
> Am 08/11/2022 um 09:20 schrieb Dominik Csapak:
> >> is this outdated due to being from april? or do we really share the
> >> ID namespace between all plugin types?
> > 
> > thats normal for section configs, but usually we don't notice in pve
> > since we normally only have a single endpoint for listing all objects
> > and not per 'type'
> > (e.g. you can't have 2 storages with the same ids but different types
> > either)
> 
> @wolfgang didn't you improve on that somewhere by some xyz-id prefix or the
> like?

I don't think so, not for the jobs api anyway. There may have been
off-list discussions?

If we really want to keep a single `jobs.cfg` for multiple types, maybe
we should also forbid accessing the jobs directly via
`$jobs->{ids}->{$id}` and require going over accessors that require a
type parameter.
(Maybe bless the $jobs hash into a dedicated package for convenience, so
we can do $jobs->get($id, $type).)

> 
> >>> +        foreach my $k (keys %$param) {
> >>
> >> probably just copy "error", but please: s/foreach/for/, or even:
> >>
> >> $job->{$_} = $param->{$_} for keys $param->%*;
> > 
> > mhmm.. AFAIR i did not see that pattern anywhere yet in our codebase, maybe we want
> > an example of that in our style guide? (for single line loops i like it)

The style guide actually already specifies the preference of `for` over
`foreach` ;-)