From: Dominik Csapak <d.csapak@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate
Date: Mon, 30 Oct 2023 12:05:58 +0100 [thread overview]
Message-ID: <a86c676b-39ec-ad35-5ff7-747cc69bddf8@proxmox.com> (raw)
In-Reply-To: <9f41a51d-e6ca-49bf-a959-e989d3ec8bad@proxmox.com>
On 10/25/23 18:38, Thomas Lamprecht wrote:
> Would name it "proxmox-rest-server-config" or "proxmox-api-config", and here
> I'm asking myself (without really trying to answer it myself): why isn't
> this part of proxmox-rest-server, it's only relevant when used in combination
> of there, simply to allow use-sites that do not depend on the rest of that
> crates code?
>
you're right that it's currently only used together with the
rest-server, but i though it might bite us in the future if we put that
together. We might want to have some daemons that does not have a rest
part in the future?
e.g. like we now have a few of them in PVE (pvestatd, qmeventd, spiceproxy)
also having more smaller crates does improve compile time for boxes
with many cores (not really an argument for or against, just a side effect)
> Also, mostly important for the other two crates: did you check the history
> of the files you mode? If there's anything relevant, I'd favor isolating that
> and then merging it in, like I did when splitting out rest-server, but it can
> sometimes be a bit of a PITA, so can be decided case-by-case for those new
> crates..
>
i'll look into it. I did not spend much time on such things for the RFC.
just wanted to see how others feel about the general approach
> Am 18/10/2023 um 12:39 schrieb Dominik Csapak:
>
>> diff --git a/proxmox-server-config/src/lib.rs b/proxmox-server-config/src/lib.rs
>> new file mode 100644
>> index 0000000..8377978
>> --- /dev/null
>> +++ b/proxmox-server-config/src/lib.rs
>> @@ -0,0 +1,302 @@
>> +//! A generic server config abstraction
>> +//!
>> +//! Used for proxmox daemons to have a central point for configuring things
>
> "Used for API daemons of Proxmox projects as a central point for configuring their
> base environment, like ..."
>
> (Proxmox is our TM and is not a project itself)
yes, of course
>
>> +//! like base/log/task/certificate directories, user for creating files, etc.
>> +
>> +use std::os::linux::fs::MetadataExt;
>> +use std::path::{Path, PathBuf};
>> +use std::sync::OnceLock;
>> +
>> +use anyhow::{format_err, Context, Error};
>> +use nix::unistd::User;
>> +
>> +use proxmox_sys::fs::{create_path, CreateOptions};
>
> this alone here is probably causing 90% compile time ^^
> we really need to split proxmox-sys more, but that's orthogonal to this
> series
>
true, IMHO at least the 'fs' part would warrant it's own crate
>> +
[snip]
>> +pub struct ServerConfig {
>
> Lacks doc comments for members, please add some even if they're
> private.
sure
>
>> + name: String,
>> + base_dir: PathBuf,
>> + user: User,
>> + privileged_user: OnceLock<User>,
>> +
>> + task_dir: OnceLock<PathBuf>,
>
> not sure if this should be an option, e.g., for simple CRUD
> API daemons there might not be any worker tasks at all.
>
that's one reason i used OnceLock, since it's only created when either
one sets it manually, or access it the first time
but Option would work too ofc if that's preferred
>
>> + log_dir: OnceLock<PathBuf>,
>
> maybe: access_log_dir (as this is the api access log that other
if we move it to rest server i agree, but otherwise i'd leave it more
generic (other daemons might have logging needs besides the access log)
>
>> + cert_dir: OnceLock<PathBuf>,
>> + state_dir: OnceLock<PathBuf>,
>> + run_dir: OnceLock<PathBuf>,
>> + config_dir: OnceLock<PathBuf>,
>> +}
>> +
>
next prev parent reply other threads:[~2023-10-30 11:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 10:39 [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 1/3] new proxmox-server-config crate Dominik Csapak
2023-10-25 16:38 ` Thomas Lamprecht
2023-10-30 11:05 ` Dominik Csapak [this message]
2023-10-30 11:41 ` Thomas Lamprecht
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 2/3] new proxmox-jobstate crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox 3/3] new proxmox-cert-management crate Dominik Csapak
2023-10-18 10:39 ` [pbs-devel] [RFC proxmox-backup 1/1] use proxmox_jobstate and proxmox_cert_management crates Dominik Csapak
2023-10-24 8:17 ` [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs Lukas Wagner
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=a86c676b-39ec-ad35-5ff7-747cc69bddf8@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=t.lamprecht@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