From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints
Date: Tue, 20 Oct 2020 12:15:54 +0200 [thread overview]
Message-ID: <20201020101554.qachhws6pfnibnwi@olga.proxmox.com> (raw)
In-Reply-To: <20201020094222.2wsepswjngyle2ru@olga.proxmox.com>
On Tue, Oct 20, 2020 at 11:42:22AM +0200, Wolfgang Bumiller wrote:
> On Mon, Oct 19, 2020 at 09:39:12AM +0200, Fabian Grünbichler wrote:
> > + let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
> > +
> > + let (mut config, expected_digest) = user::config()?;
> > +
> > + if let Some(ref digest) = digest {
> > + let digest = proxmox::tools::hex_to_digest(digest)?;
> > + crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> > + }
> > +
> > + let tokenname = Tokenname::try_from(tokenname)?;
> > + let tokenid = Userid::from((userid.name(), userid.realm(), tokenname.as_ref()));
> > +
> > + let mut data: user::ApiToken = config.lookup("token", tokenid.as_str())?;
> > +
> > + if let Some(comment) = comment {
> > + let comment = comment.trim().to_string();
> > + if comment.is_empty() {
> > + data.comment = None;
> > + } else {
> > + data.comment = Some(comment);
> > + }
> > + }
> > +
> > + if let Some(enable) = enable {
> > + data.enable = if enable { None } else { Some(false) };
>
> Really not a fan of single line if/else like this. Also with the `if
> let` together this isn't actually "fast" to read.
> How about:
>
> data.enabled = match enable {
> Some(true) => None,
> other => other,
> }
>
> or
>
> data.enabled = enable.filter(|&b| !b);
I missed that there was no `else` case which should leave it unchanged,
so only the `match` version really makes sense, but with explicit
`None => data.enabled`, maybe include comments:
data.enabled = match enable {
Some(true) => None, // enabled is default
Some(false) => Some(false),
None => data.enabled, // unchanged.
}
next prev parent reply other threads:[~2020-10-20 10:16 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-19 7:39 [pbs-devel] [RFC proxmox-backup 00/15] API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 01/15] fix indentation Fabian Grünbichler
2020-10-19 12:00 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 02/15] fix typos Fabian Grünbichler
2020-10-19 12:01 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [PATCH proxmox-backup 03/15] REST: rename token to csrf_token Fabian Grünbichler
2020-10-19 12:02 ` [pbs-devel] applied: " Thomas Lamprecht
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 04/15] Userid: extend schema with token name Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 05/15] add ApiToken to user.cfg and CachedUserInfo Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 06/15] config: add token.shadow file Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 07/15] REST: extract and handle API tokens Fabian Grünbichler
2020-10-20 8:34 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 08/15] api: add API token endpoints Fabian Grünbichler
2020-10-20 9:42 ` Wolfgang Bumiller
2020-10-20 10:15 ` Wolfgang Bumiller [this message]
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 09/15] api: allow listing users + tokens Fabian Grünbichler
2020-10-20 10:10 ` Wolfgang Bumiller
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 10/15] api: add permissions endpoint Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 11/15] client: allow using ApiToken + secret Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 13/15] tasks: allow unpriv users to read their tokens' tasks Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 14/15] manager: add token commands Fabian Grünbichler
2020-10-19 7:39 ` [pbs-devel] [RFC proxmox-backup 15/15] manager: add user permissions command Fabian Grünbichler
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=20201020101554.qachhws6pfnibnwi@olga.proxmox.com \
--to=w.bumiller@proxmox.com \
--cc=f.gruenbichler@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