search

sonic-net / DASH

Disaggregated APIs for SONiC Hosts
Apache License 2.0
81 stars 89 forks source link

Commit generated DASH SAI APIs into source control #361

Open mukeshmv opened 1 year ago

mukeshmv commented 1 year ago

DASH SAI APIs are generated from bmv2 P4 code. Any PR with P4 changes can change the generated DASH SAI APIs. However it is not easy to review these changes unless we checkout the PR in our local workspaces, build it and compare it with a copy of the previous DASH SAI APIs.

It might make it easier for a lot more people to review it if the DASH SAI API changes also showed up in the diff in the PR itself. One way of doing this would be to commit the generated APIs also to the DASH git repo. If needed this could be kept separate from the SAI submodule by placing it say under dash-pipeline/generated/ with the sole purpose of viewing the current snapshot / for review purposes so as to not disturb any of the existing workflows. This requires discipline from the developers to keep updating the folder for every PR.

This can also work as a quick reference for whoever wants to look at the current generated DASH SAI APIs since the OCP SAI repo may lag behind the latest features added in the DASH repo.

chrispsommers commented 1 year ago

@mukeshmv Excellent suggestion, and thank you for posting this. I thought about a way to ensure a developer checks in generated SAI (only if it changed): we could make a new CI compare-sai script which compares the committed, generated SAI (dash-pipeline/generated) with SAI generated during the CI run of make sai P4 (under sai/experimental) and if they disagree, it fails (or something like this). Then the PR will not get accepted. I think this would be fairly straightforward.

mukeshmv commented 1 year ago

Thanks @chrispsommers. That sounds perfect !