-
Notifications
You must be signed in to change notification settings - Fork 92
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
AppInterface enhancements for subscription #67
Conversation
Added new translateSubscribe() and processSubscribe() functions to appInterface as per HLD sonic-net/SONiC#1287 Also added stub implementation of these functions to all existing app modules. It blindly treats all paths as non-db. Basic subscription features, without on_change, can be verified with this mapping. Signed-off-by: Sachin Holla <sachin.holla@broadcom.com>
translib/acl_app.go
Outdated
} | ||
|
||
return nil, ¬ifInfo, nil | ||
err := errors.New("Not supported") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not
return tlerr.New("not implemented")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an existing code; but showing up here because gofmt converted indentation spaces into tabs. No idea why the original author chose to code it that way. I am not concentrating on such cleanups in this PR
} | ||
|
||
//App modules will use this function to register with App interface during boot up | ||
// App modules will use this function to register with App interface during boot up | ||
func register(path string, info *appInfo) error { | ||
var err error | ||
log.Info("Registering for path =", path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While correcting this file, it would be helpful to convert stuff like this to a more explicit call like log.Infof("Registering for path =%s", path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting to use Infof() instead of Info() ??
I am not performing any such cleanup of the existing code in this PR. I am only introducing translateSubscribe() & peocessSubscribe() methods and types used by them. All other changes you see (like line #98 here) are due to gofmt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for all the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Added new translateSubscribe() and processSubscribe() functions to appInterface as per HLD sonic-net/SONiC#1287
Also added stub implementation of these functions to all existing app modules. It blindly treats all paths as non-db. Basic subscription features, without on_change, can be verified with this mapping.