Skip to content

Enhance Rule#18 Disallow YANG LISTs with Over-lapping Keys#1558

Merged
wen587 merged 3 commits intosonic-net:masterfrom
wen587:rule18update
Dec 26, 2023
Merged

Enhance Rule#18 Disallow YANG LISTs with Over-lapping Keys#1558
wen587 merged 3 commits intosonic-net:masterfrom
wen587:rule18update

Conversation

@wen587
Copy link
Contributor

@wen587 wen587 commented Dec 15, 2023

Enhance Rule#18 in guideline to better describe rule requirement and make YANG LIST unique.

@wen587 wen587 marked this pull request as ready for review December 15, 2023 04:54
ganglyu
ganglyu previously approved these changes Dec 18, 2023
@wen587
Copy link
Contributor Author

wen587 commented Dec 19, 2023

@dgsudharsan could you help review?

@qiluo-msft
Copy link
Contributor

@NanQiSweeper Could you also help review?

qiluo-msft
qiluo-msft previously approved these changes Dec 20, 2023
@NanQiSweeper
Copy link

@wen587 In example 3, "keys with same number of elements and different type", if the different types here are sufficiently distinguished, is it supported? For example, both lists have only one key and different type.

typedef report-type {
    type enumeration {
        enum periodic;
        enum stream;
        enum once;
    }
}
typedef encoding {
    type enumeration {
        enum JSON_IETF;
        enum ASCII;
        enum BYTES;
        enum PROTO;
    }
}    
container EXAMPLE {
    list LIST_1 {  // 1st list
        key "report_type";
        leaf report_type {
            type report-type;
        }
    }
    list LIST_2 { // 2st list
        key "encoding";
        leaf encoding {
            type encoding;
        }
    }
}

@NanQiSweeper
Copy link

@NanQiSweeper Could you also help review?
Sure, I would be happy to help with the review.

@wen587
Copy link
Contributor Author

wen587 commented Dec 20, 2023

@wen587 In example 3, "keys with same number of elements and different type", if the different types here are sufficiently distinguished, is it supported? For example, both lists have only one key and different type.

typedef report-type {
    type enumeration {
        enum periodic;
        enum stream;
        enum once;
    }
}
typedef encoding {
    type enumeration {
        enum JSON_IETF;
        enum ASCII;
        enum BYTES;
        enum PROTO;
    }
}    
container EXAMPLE {
    list LIST_1 {  // 1st list
        key "report_type";
        leaf report_type {
            type report-type;
        }
    }
    list LIST_2 { // 2st list
        key "encoding";
        leaf encoding {
            type encoding;
        }
    }
}

Hi @NanQiSweeper , I believe no.
When using GCU(Generic Config Updater) to add or remove entry, it always match to the LIST_1 as it only compares the length of key.

@NanQiSweeper
Copy link

@wen587 In example 3, "keys with same number of elements and different type", if the different types here are sufficiently distinguished, is it supported? For example, both lists have only one key and different type.

typedef report-type {
    type enumeration {
        enum periodic;
        enum stream;
        enum once;
    }
}
typedef encoding {
    type enumeration {
        enum JSON_IETF;
        enum ASCII;
        enum BYTES;
        enum PROTO;
    }
}    
container EXAMPLE {
    list LIST_1 {  // 1st list
        key "report_type";
        leaf report_type {
            type report-type;
        }
    }
    list LIST_2 { // 2st list
        key "encoding";
        leaf encoding {
            type encoding;
        }
    }
}

Hi @NanQiSweeper , I believe no. When using GCU(Generic Config Updater) to add or remove entry, it always match to the LIST_1 as it only compares the length of key.

@wen587 @qiluo-msft I get it, I have no more questions.

@wen587 wen587 dismissed stale reviews from qiluo-msft and ganglyu via b31615d December 20, 2023 07:29
@wen587
Copy link
Contributor Author

wen587 commented Dec 20, 2023

@sachinholla Hi thanks for you reveiw. Updated in lastest commit. Please help take a look.

@wen587 wen587 merged commit ef6d015 into sonic-net:master Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants