From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Subject: Re: [pve-devel] [PATCH access-control 1/1] add realmsync plugin for jobs and CRUD api for domainsync-jobs
Date: Tue, 8 Nov 2022 10:24:50 +0100 [thread overview]
Message-ID: <0029248b-d216-edcb-51f5-d5157ab871dd@proxmox.com> (raw)
In-Reply-To: <84715fe7-760d-db37-9cc1-73c431533520@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.
next prev parent reply other threads:[~2022-11-08 9:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-04 8:54 [pve-devel] [PATCH access-control/manager] add realm sync jobs Dominik Csapak
2022-04-04 8:54 ` [pve-devel] [PATCH access-control 1/1] add realmsync plugin for jobs and CRUD api for domainsync-jobs Dominik Csapak
2022-11-07 17:05 ` Thomas Lamprecht
2022-11-08 8:20 ` Dominik Csapak
2022-11-08 9:24 ` Thomas Lamprecht [this message]
2022-11-08 10:36 ` Wolfgang Bumiller
2022-04-04 8:54 ` [pve-devel] [PATCH manager 1/4] Jobs: provide id and schedule to the job Dominik Csapak
2022-11-07 16:14 ` [pve-devel] applied: " Thomas Lamprecht
2022-04-04 8:54 ` [pve-devel] [PATCH manager 2/4] Jobs/Plugin: remove 'vzdump' from id description Dominik Csapak
2022-11-07 16:14 ` [pve-devel] applied: " Thomas Lamprecht
2022-04-04 8:54 ` [pve-devel] [PATCH manager 3/4] Jobs: add RealmSync Plugin and register it Dominik Csapak
2022-11-07 16:14 ` [pve-devel] applied: " Thomas Lamprecht
2022-11-07 16:15 ` Thomas Lamprecht
2022-04-04 8:54 ` [pve-devel] [PATCH manager 4/4] ui: add Realm Sync panel Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0029248b-d216-edcb-51f5-d5157ab871dd@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox