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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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?

>>> +        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)

$ grep -r '\$_.*for ' /usr/share/perl5/PVE
/usr/share/perl5/PVE/API2.pm:   $res->{$_} = $version_info->{$_} for qw(version release repoid);
/usr/share/perl5/PVE/CLI/pve6to7.pm:    $total += $_ for values %$counters;
/usr/share/perl5/PVE/API2/Ceph/Pools.pm:        delete $options->{$_}->{default} for keys %$options;
/usr/share/perl5/PVE/API2/Ceph/MDS.pm:      $mds_hash->{$name}->{$_} = $d->{$_} for keys %$d;
/usr/share/perl5/PVE/API2/ClusterConfig.pm:             $err_hash->{"${type}$_"} = $arr[$_] for 0..$#arr;
/usr/share/perl5/PVE/API2/Qemu.pm:                  $conf->{$_} = $created_opts->{$_} for keys $created_opts->%*;
/usr/share/perl5/PVE/API2/Qemu.pm:                  $conf->{pending}->{$_} = $created_opts->{$_} for keys $created_opts->%*;
/usr/share/perl5/PVE/API2/Disks/ZFS.pm:         eval { PVE::Diskmanage::wipe_blockdev($_) for $to_wipe->@*; };
/usr/share/perl5/PVE/API2/Cluster.pm:       $conf->{$_} = $param->{$_} for keys $param->%*;
/usr/share/perl5/PVE/API2/Cluster.pm:       delete $conf->{$_} for PVE::Tools::split_list($delete);
/usr/share/perl5/PVE/API2/Network.pm:           $ifaces->{$_} = $vnets->{$_} for keys $vnets->%*
/usr/share/perl5/PVE/API2/LXC.pm:                       $rpcenv->warn($errors->{$_}) for keys $errors->%*;
/usr/share/perl5/PVE/Service/pvescheduler.pm:   $old_workers .= "$type:$_;" for keys $worker->%*;
/usr/share/perl5/PVE/VZDump.pm:delete $confdesc_for_defaults->{$_}->{requires} for qw(notes-template protected);
/usr/share/perl5/PVE/VZDump.pm: delete $opts->{$_} for qw(notes-template protected);
/usr/share/perl5/PVE/VZDump/LXC.pm:     $task->{size} += $_->{size} for @$res;
/usr/share/perl5/PVE/Diskmanage.pm:     $_ =~ s|cciss/|cciss!| for @$disks;
/usr/share/perl5/PVE/Diskmanage.pm:         $disklist->{$_} = $partitions->{$_} for keys %{$partitions};
/usr/share/perl5/PVE/INotify.pm:            $ifaces->{$_}->{autostart} = 1 for split (/\s+/, $2);
/usr/share/perl5/PVE/INotify.pm:            $ifaces->{$_}->{autostart} = 1 for split (/\s+/, $2);
/usr/share/perl5/PVE/INotify.pm:            $d->{"$_$suffix"} = $f->{$_} for keys $f->%*;
/usr/share/perl5/PVE/QemuServer/ImportDisk.pm:          warn "hotplugging imported disk '$_' failed: $errors->{$_}\n" for keys %$errors;
/usr/share/perl5/PVE/ACME.pm:    $self->{$_} = $data->{$_} for @SAVED_VALUES;
/usr/share/perl5/PVE/ACME.pm:   print("(meta): $_ : $meta->{$_}\n") for sort keys %$meta;
/usr/share/perl5/PVE/ACME.pm:    print("$_ : $methods->{$_}\n") for sort grep {$_ ne 'meta'} keys %$methods;
/usr/share/perl5/PVE/DAB.pm:    $exclude->{$_} = 1 for split(',', $opts->{exclude});
/usr/share/perl5/PVE/Tools.pm:    check_mail_addr($_) for $mailto->@*;
/usr/share/perl5/PVE/QemuMigrate.pm:    delete $conf->{$_} for keys %$target_drives;

>>> +package PVE::Jobs::RealmSync;
>>
>> Hmm, why not DomainSync, or why is the other module called Domainsynced, no biggie but
>> slightly odd
> 
> i can't really remember, but i guess i decided on a different name sometime during
> development, and forgot to update all instances...
> 
> i'm not bound to any of these names, but i also would like to have it consistent
> (so either all DomainSync or all RealmSync)

let's do Realm, matches the frontend and in the PVE/internet world a bit less overused.