Skip to content

[meta] Skip validation for mandatory deprecated attributes in Meta.#1871

Merged
kcudnik merged 1 commit intosonic-net:masterfrom
ksravani-hcl:is_deprecated
Apr 29, 2026
Merged

[meta] Skip validation for mandatory deprecated attributes in Meta.#1871
kcudnik merged 1 commit intosonic-net:masterfrom
ksravani-hcl:is_deprecated

Conversation

@ksravani-hcl
Copy link
Copy Markdown
Contributor

@ksravani-hcl ksravani-hcl commented Apr 24, 2026

What i did:
Updated the generic object creation validation logic in Meta.cpp to check the isdeprecated flag of SAI attributes. If an attribute is marked as both mandatory and deprecated, skips the requirement check instead of potentially failing the operation.

Why i did:
In SAI, some legacy attributes are marked as mandatory for backward compatibility but are simultaneously deprecated in favor of newer mechanisms. Enforcing mandatory checks on deprecated attributes can cause unnecessary validation failures in orchestration agents that have already migrated to modern attributes, hindering smooth transitions and system stability.

How i did:
Verified the persistence mechanism through the local build and UT.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@kishanps
Copy link
Copy Markdown

@ksravani-hcl @sreeiyer-1 Please fix the verification part in the description notes, there is no Bazel env here.

@mholankar
Copy link
Copy Markdown

@kcudnik can you please approve this PR?

@ksravani-hcl ksravani-hcl marked this pull request as ready for review April 28, 2026 01:44
@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Apr 28, 2026

in description you said that if attribute is depreacated then LOG will be added, but no log message exists in this PR, also was this discussed further ? did all vendors agreed to skip this or all depreacated attributes from specifisc SAI version ?

@ksravani-hcl
Copy link
Copy Markdown
Contributor Author

in description you said that if attribute is depreacated then LOG will be added, but no log message exists in this PR, also was this discussed further ? did all vendors agreed to skip this or all depreacated attributes from specifisc SAI version ?

Corrected the log. This is discussed only internally but the change is applicable only to SAI_TUNNEL_TERM_TABLE_ENTRY_ATTR_VR_ID in saitunnel.h (https://github.com/opencomputeproject/SAI/blob/c72635e6ca35371ba0bd82d36a5b2c500c021e16/inc/saitunnel.h#L995) which is the only attribute in the SAI/inc that is both mandatory and deprecated.

@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Apr 28, 2026

i would add warning line or at least NOTICE in this case taht any attribute as depreacated, as i sspect not all vendors will initially support that right away

Comment thread meta/Meta.cpp
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik
kcudnik previously approved these changes Apr 28, 2026
Comment thread meta/Meta.cpp Outdated
Signed-off-by: SRAVANI KANASANI <kanasanis@google.com>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@ksravani-hcl
Copy link
Copy Markdown
Contributor Author

@kcudnik
as all the checks are successful, Could you please merge this PR.

@kcudnik kcudnik merged commit a7a06eb into sonic-net:master Apr 29, 2026
19 checks passed
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.

6 participants