Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAI bulk add/remove implementation #246

Merged
merged 1 commit into from May 24, 2023
Merged

Conversation

marian-pritsak
Copy link
Collaborator

Signed-off-by: Marian Pritsak marianp@mellanox.com

@marian-pritsak
Copy link
Collaborator Author

Needs a unit test before converting from draft

@chrispsommers chrispsommers self-requested a review October 5, 2022 23:58
@chrispsommers
Copy link
Collaborator

LGTM, but I'll wait for some tests and try them out myself.

@chrispsommers
Copy link
Collaborator

chrispsommers commented Feb 1, 2023

Hi @marian-pritsak any chance we can get a unit test or two to validate this and accept it?

@KrisNey-MSFT
Copy link
Collaborator

@marian-pritsak
Waiting for test cases as part of this PR

@chrispsommers
Copy link
Collaborator

@vmytnykx pointed out that the saithrift codegen (in SAI repo) lacks bulk support so it's unlikely we can use these bulk APIs in any saithrift tests in the near term.

@chrispsommers pointed out we could write tests in c++ e.g. under tests/libsai. He also said the risk was low to accept w/o a test case so we can merge and handle any surprises later. Consensus seemed to be to merge it and move on.

@chrispsommers chrispsommers marked this pull request as ready for review March 16, 2023 17:47
@chrispsommers
Copy link
Collaborator

Hi @marian-pritsak we've agreed to merge but there is a "status" issue. I don't understand it.

@KrisNey-MSFT
Copy link
Collaborator

Git command 'amend' and git push force - rinse and repeat? Vincent has seen this before...
git commit-- amend

@KrisNey-MSFT
Copy link
Collaborator

@marian-pritsak to update tomorrow 3/30/2023

Signed-off-by: Marian Pritsak <marianp@mellanox.com>
@lguohan lguohan merged commit f0517f9 into sonic-net:main May 24, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔬Scoping
Development

Successfully merging this pull request may close these issues.

None yet

4 participants