Hi Thomas,
On Fri, Jul 14, 2023 at 10:48:53AM +0200, Thomas Haller wrote:
> Note that the corresponding API for output flags does not expose the
> plain numeric flags. Instead, it exposes the underlying, flag-based C
> API more directly.
>
> Reasons:
>
> - a flags property has the benefits that adding new flags is very light
> weight. Otherwise, every addition of a flag requires new API. That new
> API increases the documentation and what the user needs to understand.
> With a flag API, we just need new documentation what the new flag is.
> It's already clear how to use it.
>
> - opinionated, also the usage of "many getter/setter API" is not have
> better usability. Its convenient when we can do similar things (setting
> a boolean flag) depending on an argument of a function, instead of
> having different functions.
>
> Compare
>
> ctx.set_reversedns_output(True)
> ctx.set_handle_output(True)
>
> with
>
> ctx.ouput_set_flags(NFT_CTX_OUTPUT_REVERSEDNS | NFT_CTX_OUTPUT_HANDLE)
>
> Note that the vast majority of users of this API will just create one
> nft_ctx instance and set the flags once. Each user application
> probably has only one place where they call the setter once. So
> while I think flags have better usability, it doesn't matter much
> either way.
>
> - if individual properties are preferable over flags, then the C API
> should also do that. In other words, the Python API should be similar
> to the underlying C API.
>
> - I don't understand how to do this best. Is Nftables.output_flags
> public API? It appears to be, as it has no underscore. Why does this
> additional mapping from function (get_reversedns_output()) to name
> ("reversedns") to number (1<<0) exist?
I don't recall why I chose to add setters/getters for individual output
flags instead of expecting users to do bit-fiddling. Maybe the latter is
not as common among Python users. :)
On the other hand, things are a bit inconsistent already, see
set_debug() method.
Maybe we could turn __{get,set}_output_flag() public and make them
accept an array of strings or numbers just like set_debug()? If you
then adjust your input flag API accordingly, things become consistent
(enough?), without breaking existing users.
FWIW, I find
| ctx.set_output_flags(["reversedns", "stateless"])
nicer than
| ctx.set_output_flags(REVERSEDNS | STATELESS)
at least with a Python hat on. WDYT?
Cheers, Phil
