Similarityoung commented on code in PR #688:
URL: https://github.com/apache/dubbo-go-pixiu/pull/688#discussion_r2175138998
##########
pkg/common/extension/filter/filter.go:
##########
@@ -97,6 +99,15 @@ type (
OnData(data any) (any, error)
// OnTripleData triple rpc invocation from triple-server
OnTripleData(ctx context.Context, methodName string, arguments
[]any) (any, error)
+ // OnUnaryRPC handles a unary RPC call.
+ OnUnaryRPC(ctx context.Context, fullMethod string, req any)
(any, error)
+ // OnStreamRPC handles a streaming RPC call.
+ // In gRPC, a Unary call is fundamentally a special,
short-lived case of a stream.
+ // By consolidating all handling here, we leverage a single,
robust logic path for all
+ // gRPC proxying, simplifying the architecture and ensuring
consistent behavior.
+ OnStreamRPC(stream model.RPCStream, info *model.RPCStreamInfo)
error
+ // Close the filter and release resources
+ Close() error
Review Comment:
That's a great point, and I completely agree. Splitting the interface and
using composition is definitely more idiomatic Go and better for long-term
maintenance.
I took an initial look, and it seems the refactoring for this is quite
significant and would go beyond the scope of this current PR.
To address it properly, would it be acceptable to merge this PR first? I can
then immediately open a dedicated follow-up PR to refactor the NetworkFilter
interface.
Please let me know if this approach works for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
