Skip to content

Add Debug Operation - #33

Draft
asafsonnv wants to merge 1 commit into
openconfig:mainfrom
asafsonnv:debug
Draft

Add Debug Operation#33
asafsonnv wants to merge 1 commit into
openconfig:mainfrom
asafsonnv:debug

Conversation

@asafsonnv

Copy link
Copy Markdown
Contributor

@asafsonnv

Copy link
Copy Markdown
Contributor Author

Hi:)
Should we implement the Execute() for Debug() to send and receive messages from the server?
for sending I see some logic that sends that opens the stream and sends messages in debug_grpc.pb.go:
func (c *debugClient) Debug(ctx context.Context, in *DebugRequest, opts ...grpc.CallOption) (Debug_DebugClient, error) { stream, err := c.cc.NewStream(ctx, &Debug_ServiceDesc.Streams[0], "/gnoi.debug.Debug/Debug", opts...) if err != nil { return nil, err } x := &debugDebugClient{stream} if err := x.ClientStream.SendMsg(in); err != nil { return nil, err } if err := x.ClientStream.CloseSend(); err != nil { return nil, err } return x, nil }

currently the Execute() returns a DebugClient.

@asafsonnv

Copy link
Copy Markdown
Contributor Author

For the debug_test.go:
should I add a fake server to return a message?

@asafsonnv

Copy link
Copy Markdown
Contributor Author

@greg-dennis WDYT?

@greg-dennis greg-dennis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not involved in the project anymore, so I don't have any kind of meaningful say over this. I made a few small comments and overall it looks alright, but you'll have to leave it to the people who are involved to say for sure.

Comment thread debug/debug.go
@@ -0,0 +1,81 @@
// Copyright 2023 Google LLC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2024

Comment thread gnoigo.go
OTDR() otpb.OTDRClient
System() spb.SystemClient
WavelengthRouter() wrpb.WavelengthRouterClient
DEBUG() dbgpb.DebugClient

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should just be stylized as "Debug" not "DEBUG" because it isn't an intiailism.

Comment thread internal/clients.go
}

// Debug returns the client for gNOI Debug service.
func (c *Clients) DEBUG() dbgpb.DebugClient {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"DEBUG" -> "Debug"

Comment thread debug/debug.go

// Mode the commands will be executed in.
//
// enum Mode {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Putting the full enum in the comment creates a double-maintenance burden. I'd just let people look it up in the proto.

@asafsonnv

asafsonnv commented Dec 4, 2024

Copy link
Copy Markdown
Contributor Author

@dplore Are you the current maintainer of this project?

@asafsonnv

Copy link
Copy Markdown
Contributor Author

@prinikasn may you can assist?

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.

2 participants