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

[sonic-db-cli]: SONiC DB CLI for DPU objects #837

Closed
wants to merge 12 commits into from

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Nov 21, 2023

Wait for PR: sonic-net/sonic-swss#2966
Read DPU objects from Redis via SONiC DB CLI

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur
Copy link
Contributor Author

/azpw run Azure.sonic-swss-common

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss-common

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur Pterosaur changed the title dpu db support by cli [sonic-db-cli]: SONiC DB CLI for DPU objects Nov 29, 2023
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur marked this pull request as ready for review December 2, 2023 13:20
@@ -94,22 +94,24 @@ class RedisReply

std::string to_string();

static std::string to_string(redisReply *reply, std::string command = std::string());
static std::string to_string(redisReply *reply,const std::string &command = std::string());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const

Please add one blank before const.


// restore binary pb message to TEST_DB
EXPECT_EQ(system("cat ./tests/cli_test_data/pb_appliance.bin "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pb_appliance.bin

Do you want to check-in this binary file with this PR? Better to prevent binary file in codebase, instead you can generate it.

@@ -93,6 +93,15 @@ CFLAGS_COMMON+=" -fstack-protector-strong"

AC_SUBST(CFLAGS_COMMON)

AC_CHECK_LIB([dashapi], [PbBinaryToJsonString], [
AC_DEFINE(DASH_API_INSTALLED, 1, [Define if DASH API is installed])
LIBS="$LIBS -ldashapi"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ldashapi

swss-common is widely used, it is awkward to ask others to install dashapi.
I see below options:

  1. remove the dependency on dashapi, and make this PR a generic feature
  2. refactor sonic-db-cli to support plugin, and develop a new plugin with dashapi dependency
  3. develop another CLI like sonic-db-dash-cli (just random name).

@Pterosaur Pterosaur marked this pull request as draft December 5, 2023 08:59
@Pterosaur
Copy link
Contributor Author

Refactor PR: sonic-net/sonic-dash-api#12

@Pterosaur Pterosaur closed this Dec 7, 2023
qiluo-msft pushed a commit that referenced this pull request Jan 25, 2024
Add SAG table definition for static anycast gateway.
Static anycast gateway HLD [#837](sonic-net/SONiC#837)
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.

3 participants