Re: [core] implementer feedback on CORE-SID: Re: [mbj4668/pyang] Sid sx structure (PR #839)

Jernej Tuljak <jernej.tuljak@mg-soft.si> Fri, 24 March 2023 13:49 UTC

Return-Path: <jernej.tuljak@mg-soft.si>
X-Original-To: core@ietfa.amsl.com
Delivered-To: core@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 42AACC14CE4C for <core@ietfa.amsl.com>; Fri, 24 Mar 2023 06:49:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.097
X-Spam-Level:
X-Spam-Status: No, score=-2.097 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=mg-soft.si
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LmTB-WL6LhVe for <core@ietfa.amsl.com>; Fri, 24 Mar 2023 06:49:09 -0700 (PDT)
Received: from galileo.mg-soft.si (gate.mg-soft.si [212.30.73.66]) by ietfa.amsl.com (Postfix) with ESMTP id 4132AC14CE45 for <core@ietf.org>; Fri, 24 Mar 2023 06:49:07 -0700 (PDT)
Received: from [10.0.0.222] (tp-x61t.mg-soft.si [10.0.0.222]) by galileo.mg-soft.si (Postfix) with ESMTP id 7124AC41D787; Fri, 24 Mar 2023 14:49:05 +0100 (CET)
DKIM-Filter: OpenDKIM Filter v2.11.0 galileo.mg-soft.si 7124AC41D787
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mg-soft.si; s=default; t=1679665745; bh=CHlQ1kr1U5uocLZ7FoHR6TthshFrFkGmJe/xJE5ry88=; h=Date:From:Subject:To:Cc:References:In-Reply-To:From; b=ZmvBRTkvLwL8cv1d/KHXWygK/futxHrfhggPFEmuoQQtQSXyUIZQHlh9FzC+HvM7x VBbVTkqk5WFbcDfBwb6ftTXe37MlUo6I60+oWNCa6C4Ewzf/A7CvhKZ7sKbrlQvlg7 3BPRPXpDiZoeGW0Qy8jru1u0Brc9gvuFJsBLLVP/v6hc2S6ONriZxBbcm16NFm4vLL 1/Ra4Fj9RFFooAdJBcUUXlMY0WZmnR8zoxQ143ooLDc9wpuN/NLffp1cPWZKbtzQf7 c7WAoGxananYkX2PlG4E0wN6Ii9D57+M+bMR21DMIOvTWR1BryElfn5CinAeCyjXAy 3R1Sx2FTc81dw==
Message-ID: <fb260343-b552-ff15-7665-2033ca209120@mg-soft.si>
Date: Fri, 24 Mar 2023 14:49:05 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0
From: Jernej Tuljak <jernej.tuljak@mg-soft.si>
To: Michael Richardson <mcr+ietf@sandelman.ca>
Cc: core@ietf.org
References: <mbj4668/pyang/pull/839@github.com> <mbj4668/pyang/pull/839/c1474951745@github.com> <2897262.1679232828@dyas> <bed2e140-d5e3-2aba-c2e1-6c8066602660@mg-soft.si> <28078.1679313444@localhost> <ea2899eb-e02c-9ad0-2ead-c0a341410225@mg-soft.si> <4633.1679579478@localhost>
Content-Language: en-US
In-Reply-To: <4633.1679579478@localhost>
Content-Type: text/plain; charset="UTF-8"; format="flowed"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/9EssUubpllHK8Lfku8K1r4TgcoQ>
Subject: Re: [core] implementer feedback on CORE-SID: Re: [mbj4668/pyang] Sid sx structure (PR #839)
X-BeenThere: core@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/core>, <mailto:core-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/core/>
List-Post: <mailto:core@ietf.org>
List-Help: <mailto:core-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/core>, <mailto:core-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 24 Mar 2023 13:49:14 -0000

On 23/03/2023 14:51, Michael Richardson wrote:
> Jernej Tuljak<jernej.tuljak@mg-soft.si>  wrote:
>      > On 20/03/2023 12:57, Michael Richardson wrote:
>      >> Jernej Tuljak<jernej.tuljak@mg-soft.si>  wrote:
>      >> > The issue I stumbled upon is not the number of mappings, but how the actual
>      >> > mapping for rc:yang_data is done after my changes compared to master.
>      >>
>      >> You have introduced a possible instability in assignments :-)
>      >> I can live with that as a one-time hit if it makes the code better, but are
>      >> you sure that your code is itself stable?  That might mean sorting keys of a
>      >> dict before using them.
>
>      > I'm not sure I understand what you meant. Were you referring to the change in
>      > the order of assignments for schema nodes? Yes, those occur in "pyang
>
> I'm asking if your new order of assignment would be stable even if the dict()
> implementation was changed, or if it ran on a different architecture.

It relies on collections.OrderedDict, same as the original 
implementation. That should be in insertion order on any platform. You 
are asking if the same SIDs would be assigned to items of the same YANG 
module, no matter where this plugin is run. I think this is the case - 
provided that the version of pyang is the same. The only way to know for 
sure would be to test for this. If this is a problem in my 
implementation, it is also a problem in the original one.

>      > I did go through all the tests to make sure nothing's breaking other than in
>      > expected ways, but the amount of cases those check is limited. That is also
>      > the only measure of stability I can provide at this time. I pushed (to my
>      > repo) the changes that would be required for test/test_sid yesterday.
>
> Thank you for going through those tests.

I've made more changes on my fork. The original output had a lot of 
issues that needed to be mended. Here's some of those: there was no 
"ietf-sid-file:sid-file" wrapper object present in JSON output, lists 
were encoded with an "s" suffix ("items" instead of "item", for 
example), "dependency-revision" was not encoded at all, valid leafs such 
as "description" , "sid-file-version", etc. were being rejected as 
invalid if present in the .sid file, it was never considered that JSON 
object keys could appear in their qualified form (with an 
"ietf-sid-file" prefix; "ietf-sid-file:item"), ...

I've also implemented crude validation of a .sid file against the .yang 
for which the .sid was generated. All items are looked up in the YANG 
file, including schema node identifiers for the data namespace. One can 
do that with --sid-check-file-valid option.

Files that were generated with the original implementation may be 
converted with --sid-update-file in a way that should keep all the 
existing mappings and only assign some new ones.

A thing that I did not address are the changes related to "sid-file-status".

Jernej

> --
> ]               Never tell me the odds!                 | ipv6 mesh networks [
> ]   Michael Richardson, Sandelman Software Works        |    IoT architect   [
> ]mcr@sandelman.ca   http://www.sandelman.ca/         |   ruby on rails    [
>