[pve-devel] [PATCH access-control 1/1] add realmsync plugin for jobs and CRUD api for domainsync-jobs
Wolfgang Bumiller
w.bumiller at proxmox.com
Tue Nov 8 11:36:03 CET 2022
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` ;-)
More information about the pve-devel
mailing list