public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] [PATCH common] properly encode YAML via YAML::XS
Date: Fri, 18 Sep 2020 09:13:07 +0200	[thread overview]
Message-ID: <1600412785.57pvbm8v64.astroid@nora.none> (raw)
In-Reply-To: <27b2c399-41fb-6e0e-fa27-2fb05a31adf2@proxmox.com>

On September 17, 2020 5:06 pm, Thomas Lamprecht wrote:
> On 9/17/20 1:16 PM, Fabian Grünbichler wrote:
>> otherwise we get strange errors when formatting data that was originally
>> JSON, and can thus contain JSON::true/JSON::false.
>> 
>> one example is the QMP query-blockstats command, which gets called (and
>> the resulting values returned) by /nodes/NODE/qemu/VMID/status/current
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> 
>> Notes:
>>     alternatives include:
>>     - dropping --output-format yaml altogether
> 
> FWIW: I like yaml, but we do not have it yet in PBS.
> It's at least not an option for the 6.x release.
> 
>>     - manually recursively mapping JSON::true/false to some sensible value before dumping
> 
> hmm, could do, not ideal, mostly because we then recurse everything
> and the outputter does another time - feels not good.
> 
>>     - outputting JSON instead of YAML, since the former is a subset of the latter (thanks Dominik ;))
> 
> NAK, while technical true I see no point in doing that. yaml is used
> over JSON for being more concise and having less syntax noise getting
> in ones way when reading it.
> 
> Was this issued raised on the currently used module's upstream?
> Maybe we/they could fix it there too, helping more than just our use
> case.

not raised, but given the docs/description I'd say chances are rather 
slim:

    This module implements a subset of the YAML specification for use in
    reading and writing CPAN metadata files like META.yml and MYMETA.yml. It
    should not be used for any other general YAML parsing or generation
    task.

it's based on/derived from YAML::Tiny, which states:

    It only supports a very basic subset of the full YAML specification.

    Usage is targeted at files like Perl's META.yml, for which a small and
    easily-embeddable module is extremely attractive.

    Features will only be added if they are human readable, and can be
    written in a few lines of code. Please don't be offended if your request
    is refused. Someone has to draw the line, and for YAML::Tiny that
    someone is me.

> 
> 
> That said, I have no real objection against using this XS binding of
> libyaml-0-2.
> btw. we get that already installed on ceph setups through the dependency
> chain: ceph-mgr -> python3-yaml -> libyaml-0-2

install size is also very small (xs+lib are ~200kb), memory overhead 
probably quite a bit more? we could load it only in the code-path where 
we render yaml ;)




  reply	other threads:[~2020-09-18  7:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-17 11:16 Fabian Grünbichler
2020-09-17 15:06 ` Thomas Lamprecht
2020-09-18  7:13   ` Fabian Grünbichler [this message]
2020-09-18 12:35     ` Thomas Lamprecht
2020-09-18 12:36 ` [pve-devel] applied: " Thomas Lamprecht

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=1600412785.57pvbm8v64.astroid@nora.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pve-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal