From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [RFC proxmox/proxmox-backup] refactor common server code from pbs
Date: Wed, 18 Oct 2023 12:39:07 +0200 [thread overview]
Message-ID: <20231018103911.3798182-1-d.csapak@proxmox.com> (raw)
# General
Sending this as RFC because of the multiple ways (see 'Alternatives'
section) we could implement this (or something similar). I'm not sure
what the general opinion about this pattern is.
# Goal
When creating a new daemon/server, we currently have much code in pbs
that would have to be duplicated, mostly relating to setting up the
server directories/files. Some crates features already have an interface
to be slightly configurable, but not all of them.
This change adds a new crate 'proxmox-server-config' that provides an
interface which should be easy to use and reusable across multiple
daemons/server applications for our use-cases.
In this series i opted to factor out the two main remaining crates that
are not seperated yet: the jobstates and the certificate management.
(Though i guess there are more, depending on what another daemon might
want to do)
# Implementation
I use a static OnceLock to save a global `ServerConfig` struct, that
contains some configurations available, which are either setup, or
derived from the minimum required configuration (base_dir, name, user).
This can then be used to access them from any point in the program or in
crates where needed.
With this, we don't have to carry around the different directories as
parameters, nor do we have to sprinkle things like `config!("foo.bar")`,
around the code to get the path we want.
# Alternatives
The main alternatives to this approaches are:
* copy the code across different server/daemons:
obviously the easiest way, but doesn't scale well and prone to
diverging implementations.
* parameters:
this has two ways of working:
- all functions must take the configuration (dir(s)/user(s))
blows up the parameters, and we have to use our current dir macros
even more
- each crate/module has an initializer that takes the config, which
would then e.g. save that into a static OnceLock once again,
disadvantage here is that we may duplicate values across multiple
crates (should not be much though)
There are probably more ways to implement this (e.g. maybe a global
trait object that must be implemented by each server?)
# Future work
We could extend this pattern for other crates (e.g. rest-server) so
that we don't have to carry around the parameters anymore.
Also we could make the ServerConfig extendable, where each server could
configure it's own necessary variables (e.g with a hashmap)
proxmox:
Dominik Csapak (3):
new proxmox-server-config crate
new proxmox-jobstate crate
new proxmox-cert-management crate
Cargo.toml | 5 +
proxmox-cert-management/Cargo.toml | 23 ++
proxmox-cert-management/debian/changelog | 5 +
proxmox-cert-management/debian/control | 53 ++++
proxmox-cert-management/debian/copyright | 18 ++
proxmox-cert-management/debian/debcargo.toml | 7 +
proxmox-cert-management/src/lib.rs | 182 +++++++++++
proxmox-jobstate/Cargo.toml | 26 ++
proxmox-jobstate/debian/changelog | 5 +
proxmox-jobstate/debian/control | 55 ++++
proxmox-jobstate/debian/copyright | 18 ++
proxmox-jobstate/debian/debcargo.toml | 7 +
proxmox-jobstate/src/api_types.rs | 40 +++
proxmox-jobstate/src/jobstate.rs | 315 +++++++++++++++++++
proxmox-jobstate/src/lib.rs | 46 +++
proxmox-server-config/Cargo.toml | 16 +
proxmox-server-config/debian/changelog | 5 +
proxmox-server-config/debian/control | 37 +++
proxmox-server-config/debian/copyright | 18 ++
proxmox-server-config/debian/debcargo.toml | 7 +
proxmox-server-config/src/lib.rs | 302 ++++++++++++++++++
21 files changed, 1190 insertions(+)
create mode 100644 proxmox-cert-management/Cargo.toml
create mode 100644 proxmox-cert-management/debian/changelog
create mode 100644 proxmox-cert-management/debian/control
create mode 100644 proxmox-cert-management/debian/copyright
create mode 100644 proxmox-cert-management/debian/debcargo.toml
create mode 100644 proxmox-cert-management/src/lib.rs
create mode 100644 proxmox-jobstate/Cargo.toml
create mode 100644 proxmox-jobstate/debian/changelog
create mode 100644 proxmox-jobstate/debian/control
create mode 100644 proxmox-jobstate/debian/copyright
create mode 100644 proxmox-jobstate/debian/debcargo.toml
create mode 100644 proxmox-jobstate/src/api_types.rs
create mode 100644 proxmox-jobstate/src/jobstate.rs
create mode 100644 proxmox-jobstate/src/lib.rs
create mode 100644 proxmox-server-config/Cargo.toml
create mode 100644 proxmox-server-config/debian/changelog
create mode 100644 proxmox-server-config/debian/control
create mode 100644 proxmox-server-config/debian/copyright
create mode 100644 proxmox-server-config/debian/debcargo.toml
create mode 100644 proxmox-server-config/src/lib.rs
proxmox-backup:
Dominik Csapak (1):
use proxmox_jobstate and proxmox_cert_management crates
Cargo.toml | 8 +
pbs-api-types/Cargo.toml | 1 +
pbs-api-types/src/jobs.rs | 41 +---
src/auth_helpers.rs | 184 +---------------
src/bin/proxmox-backup-api.rs | 8 +-
src/bin/proxmox-backup-manager.rs | 2 +
src/bin/proxmox-backup-proxy.rs | 2 +
src/server/jobstate.rs | 347 +-----------------------------
src/server/mod.rs | 21 ++
9 files changed, 52 insertions(+), 562 deletions(-)
--
2.30.2
next reply other threads:[~2023-10-18 10:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 10:39 Dominik Csapak [this message]
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
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=20231018103911.3798182-1-d.csapak@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pbs-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox