Doesn't choice and case statement in YANG result in oneof fields in proto?

[Baalajee-S]

Input YANG file:

module test {

  yang-version "1";

  namespace "urn:test";
  prefix "test";

  import openconfig-extensions { prefix oc-ext; }

  // meta
  organization "None";

  contact
    "[email protected]";

  description
    "Defines a data model for test purpose.";

  // Correct?
  oc-ext:openconfig-version "0.3.0";

  revision "2023-12-30" {
    description
      "Test";
    // Correct?
    reference "0.3.0";
  }

  container top {
    choice test-choice {
      case case-foo {
        leaf foo {
          type uint8;
        }
      }
      case case-bar {
        leaf bar {
          type boolean;
        }
      }
    }
  }
}

Output proto file:

// openconfig.test is generated by proto_generator as a protobuf
// representation of a YANG schema.
//
// Input schema modules:
//  - yang/bgpls_topology/test.yang
// Include paths:
//   - yang/...
syntax = "proto3";

package openconfig.test;

import "github.com/openconfig/ygot/proto/ywrapper/ywrapper.proto";
import "github.com/openconfig/ygot/proto/yext/yext.proto";

option go_package = "bgpls_topology_proto/openconfig/test";

message Top {
  ywrapper.BoolValue bar = 292023054 [(yext.schemapath) = "/top/bar"];
  ywrapper.UintValue foo = 298489470 [(yext.schemapath) = "/top/foo"];
}

Should the generated proto be?

// openconfig.test is generated by proto_generator as a protobuf
// representation of a YANG schema.
//
// Input schema modules:
//  - yang/bgpls_topology/test.yang
// Include paths:
//   - yang/...
syntax = "proto3";

package openconfig.test;

import "github.com/openconfig/ygot/proto/ywrapper/ywrapper.proto";
import "github.com/openconfig/ygot/proto/yext/yext.proto";

option go_package = "bgpls_topology_proto/openconfig/test";

message Top {
  oneof foo_bar {
    ywrapper.BoolValue bar = 292023054 [(yext.schemapath) = "/top/bar"];
    ywrapper.UintValue foo = 298489470 [(yext.schemapath) = "/top/foo"];
  }
}

[wenovus]

Hi Baalajee, your proposal makes sense. It would be a breaking change but it conforms to YANG's specs better.

The entrypoint to changing this would be here:

ygot/genutil/common.go

Lines 267 to 286 in f6d9cff

	// It should be noted that special handling is required for choice and case - because these are
	// directories within the resulting schema, but they are not data tree nodes. So for example,
	// we can have:
	//
	// /container/choice/case-one/leaf-a
	// /container/choice/case-two/leaf-b
	//
	// In this tree, "choice" and "case-one" (which are choice and case nodes respectively) are not
	// valid data tree elements, so we recurse down both of the branches of "choice" to return leaf-a
	// and leaf-b. Since choices can be nested (/choice-a/choice-b/choice-c/case-a), and can have
	// multiple data nodes per case, then the addNonChoiceChildren function will find the first
	// children of the specified node that are not choice or case statements themselves (i.e., leaf-a
	// and leaf-b in the above example).
	//
	// The .*ExcludeDerivedState compress behaviour options further filters the returned set of
	// children based on their YANG 'config' status. When set, then
	// any read-only (config false) node is excluded from the returned set of children.
	// The 'config' status is inherited from a entry's parent if required, as per
	// the rules in RFC6020.
	func FindAllChildren(e *yang.Entry, compBehaviour CompressBehaviour) (map[string]*yang.Entry, map[string]*yang.Entry, []error) {

Since we're maintaining ygot on a best-effort basis, I can't give any estimates on when this can be done, but you're welcome to contribute and I'd be happy to give hints and code review.

[robshakir]

Doing this behind a flag would solve the backwards incompatibility issues at least initially. Some care needs to be taken with what the naming of the `oneof` is -- but this does seem reasonable. It wasn't something I focused on when implementing this because OpenConfig doesn't use `choice`/`case`. I'm also open to helping review a contribution here. r.

…

On Thu, Jan 11, 2024 at 10:45 AM Wen Bo Li ***@***.***> wrote: Hi Baalajee, your proposal makes sense. It would be a breaking change but it conforms to YANG's specs better. The entrypoint to changing this would be here: https://github.com/openconfig/ygot/blob/f6d9cff4bdcf98553af92a5fef4932cdbc9e8444/genutil/common.go#L267-L286 Since we're maintaining ygot on a best-effort basis, I can't give any estimates on when this can be done, but you're welcome to contribute and I'd be happy to give hints and code review. — Reply to this email directly, view it on GitHub <#948 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABE43VOVAOW2MLANR7RG5GDYOAXONAVCNFSM6AAAAABBWU72VKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBXG42TONJWG4> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- robjs VRTS *TZ:* Pacific (America/Los_Angeles) *Desk:* SFO-SPE Please do not feel obligated to reply outside your working hours! *What is now proved was once only imagined - William Blake.*