[pve-devel] [PATCH manager v3 2/2] ui: only allow rbd pools to be added as rbd storage

[Thomas Lamprecht]

Am 21/10/2022 um 09:02 schrieb Stefan Sterz:
>> // filter out rgw, metadata, cephfs, ... pools
>> onlyRBDPools = ({data}) => !!data?.applications?.rbd;
>>
>>
> i see your point on variable naming, but this isn't equivalent afaiu.

argh, yeah you're right

> the idea here was that the ui reverts back to including a pool if there
> is no application defined for it (this should help make the ui change
> independent from the api change). this wouldn't do that. you could do:
> 
> onlyRBDPools = ({ data }) => !data?.applications ||
> !!data?.applications?.rbd
> 
> imo still pretty long, but maybe you have another trick up your
> sleeve 😄

maybe (untested):

onlyRBDPools = ({ data }) => (data?.applications ?? { rbd: true }).rbd;


But at that point we're rather heading in direction of code golf, so just
use whatever you think works good and is understandable enough, I won't
bikeshed the style of this fillter fn anymore ;P

> 
> if you don't care about keeping the front-end compatible with the
> previous behavior we can also use your one-liner. just wanted to err on
> the side of caution.
> 
> as a sidenote: personally i also prefer checking for undefined-ness to
> relying on its falsy nature. e.g. if application.rbd = null, this would
> also return false, even though technically rbd is defined. since ceph
> seems to return empty objects for rbd pools by default, this currently
> works fine. however, id prefer being more explicit. this is just a
> personal preference though, so i'll do whatever you prefer 😄

in general OK but with the point of not being sure what cephs defined
behaviour is, or will be, checking falsy can even be a feature not a bug,
otoh. ceph releases are controlled by us and have a relatively low frequency,
if it ever changes we'll be able to fix it just fine in time.

> 
> there is also the "in" operator, but if irc another patch of mine once
> got rejected for using that.
> 
>> more generally, the application could be also shown in our ceph pool list, could
>> be useful to see the usage type of each pool, would also reduce confusion for the
>> a bit odd metadata pool with its single PG.
>>
> sure i suppose that makes sense, i can add that to a v4.
> 

can, and probably should be in a separate series, just noticed and mentioned
before I forgot again ;-)