all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server] fix backpu/restore with ipv6/ports for pbs
Date: Thu, 3 Dec 2020 10:46:43 +0100	[thread overview]
Message-ID: <792e0100-c62e-2d26-4e8b-369032254e81@proxmox.com> (raw)
In-Reply-To: <27fff7eb-5f72-b466-9409-94dc70b550da@proxmox.com>

On 03.12.20 10:40, Dominik Csapak wrote:
>>
>> a few lines below, just out of context we see the sole use for all this extracting:
>>
>> my $repo = "$username\@$server:$datastore";
>>
>> So how about adding a get_repo sub to storage, or the PBSClient in pve-common
>> (if we have a documented datastructure like scfg to get the info sanely from).
> 
> well afaics, we use everywhere the same section config options
> (server,fingerprint,datastore,etc.) except there is no 'port'
> setting in pmg (put this code here handles it ok, and we can add it no problem)
> is that enough, or how else would you document it (or make
> sure users know what to put in here)?
> 
> would a simple comment above the helper be enough? or should we
> try to refactor those section config options across products?

yeah a short comment and maybe the structure of known keys in the $cfg
variable should be enough for us - I do not see this changing soon.

I did something like that for the pve-storage "scan_datastores" sub:
https://git.proxmox.com/?p=pve-storage.git;a=commitdiff;h=8b62ac6a0ce0d4757ce25082123d18434f5b3a58

> 
> so i'd simply add a 'build_repository' sub to pve-commons pbsclient module,
> and use that everywhere we need that
> is that ok?

Personally I'd went for "get_repository", build sounds a bit strange to me,
albeit it is not wrong (this has big bike shedding potential, so no hard
feelings from me).





      reply	other threads:[~2020-12-03  9:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03  8:43 Dominik Csapak
2020-12-03  9:27 ` Thomas Lamprecht
2020-12-03  9:40   ` Dominik Csapak
2020-12-03  9:46     ` Thomas Lamprecht [this message]

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=792e0100-c62e-2d26-4e8b-369032254e81@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal