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 ;-)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
