union: propagate apptag error if all errors use the same apptag

[bradh352]

If a union references various different leafref paths, and all leafref paths are invalid, it is nice to have the apptag returned as instance-required. There may be other use cases...

[michalvasko]

Obviously, you have this specific use-case. But if you want to change it, I suppose the app-tag can be used if all types define it or not define any.

[bradh352]

I'd view this as more of a "nice-to-have".

I would need to update some existing test cases in SONiC without this patch for expected errors, unless I wanted to go dirty and try to parse and evaluate the error message itself.

In SONiC's yang models, we do a lot of leafref unions, specifically for cross referencing physical and virtual interfaces ... like an ACL must be associated with a physical port, a port channel, or a vlan interface ... all of which are defined in different modules/schema paths.

I don't think we have any instance where we mix a union with leafref and non-leafref nodes, so not getting the "instance-required" apptag makes things a bit more ambiguous for being able to render our own error messages.

[michalvasko]

What I meant is that I will accept this PR if you change it so that an error-app-tag is used if the same or none is used for all the types. Using it only if all the types generate it seems too restrictive.

[bradh352]

What I meant is that I will accept this PR if you change it so that an error-app-tag is used if the same or none is used for all the types. Using it only if all the types generate it seems too restrictive.

Ah sorry, didn't understand that first time around. The latest update I just pushed should accomplish that.

[michalvasko]

When the tests did not pass, I decided to improve it a bit myself, should work.

[bradh352]

it was an ubsan thing, where it didn't realize apptag couldn't be null if use_apptag was set. I had gone back and fixed that. But that's fine, yours works.