On Mon, Aug 18, 2025 at 11:07:34PM +0200, Phil Sutter wrote:
> On Mon, Aug 18, 2025 at 04:16:21PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Aug 13, 2025 at 07:05:35PM +0200, Phil Sutter wrote:
> > > This series consists of noise (patches 1-13 and most of patch 14) with a
> > > bit of signal in patch 14. This is because the relatively simple
> > > adjustment to JSON output requires minor adjustments to many stored JSON
> > > dumps in shell test suite and stored JSON output in py test suite. While
> > > doing this, I noticed some dups and stale entries in py test suite. To
> > > clean things up first, I ran tests/py/tools/test-sanitizer.sh, fixed the
> > > warnings and sorted the changes into fixes for the respective commits.
> >
> > Reviewed-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
>
> Series applied, thanks!
>
> > I will follow up with a patch to partially revert the fib check change
> > for JSON too.
>
> Hmm. That one seems like a sensible change and not just a simplification
> of output.
Actually, I don't find an easy way to retain backward compatibility in
the JSON output for fib without reverting:
commit 525b58568dca5ab9998595fc45313eac2764b6b1
Author: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
Date: Tue Jun 24 18:11:10 2025 +0200
fib: allow to use it in set statements
commit f4b646032acff4d743ad4f734aaca68e9264bdbb
Author: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
Date: Tue Jun 24 18:11:06 2025 +0200
fib: allow to check if route exists in maps
I am not sure I want to do that, because then the fib expression
cannot be used with sets/maps.
The "check" trick was a "smart" workaround, I don't have a way to know
if you want to check for the presence of the real oif, without peeking
on the relational right hand side, which only works for rules without
set/map.
Peeking on the right hand side does not work to decide the semantics
of the right hand side for relationals, but not for set and maps,
because those could be empty, hence this "check" field.
Maybe, simply retain backward compatibility for the fib check with
relational expression would do the trick? But I am not sure the JSON
parser provides context on whether the expression is being used in a
relational.
> I guess if we take this approach seriously, we should agree
> on (and communicate) an upgrade path for JSON output. In detail (from
> the top of my head):
>
> 1) What changes are considered compatible (and which not)
> 2) In which situations are incompatible changes acceptable
> 3) How to inform users of the incompatible change
>
> I'd suggest something like:
>
> 1) Additions only, no changes of property values or names
> 2) Critical bug fixes or new (major?) versions
> 3) Bump JSON_SCHEMA_VERSION? Or is the "version" property in "metainfo"
> sufficient if bumped anyway?
This was going to be an issue sooner or later.
Updating the JSON representation would require to maintain backward
compatibility, changes would need to happen in a less incremental way
because you don't want to change the schema so often.
And this would also require to extend tests/ to deal with the
different versions? Which is going to be very tricky, it sounds like a
no-go.
Then all this means that everything is set in stone for third party
parsers, and that we have to forget the idea the JSON representation
can be incrementally refined, and simply add more stuff on top of it
as you suggest.
