Skip to content

AppInterface enhancements for subscription#67

Merged
tomek-US merged 7 commits intosonic-net:masterfrom
sachinholla:subscribe_app_intf
Apr 25, 2023
Merged

AppInterface enhancements for subscription#67
tomek-US merged 7 commits intosonic-net:masterfrom
sachinholla:subscribe_app_intf

Conversation

@sachinholla
Copy link
Copy Markdown
Contributor

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.

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>
Comment thread translib/acl_app.go Outdated
}

return nil, &notifInfo, nil
err := errors.New("Not supported")
Copy link
Copy Markdown

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")

Copy link
Copy Markdown
Contributor Author

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

Comment thread translib/acl_app.go Outdated
Comment thread translib/acl_app.go Outdated
Comment thread translib/common_app.go Outdated
Comment thread translib/internal/apis/db_diff.go
Comment thread translib/pfm_app.go
Comment thread translib/sys_app.go
Comment thread translib/yanglib_app.go
Comment thread translib/subscribe_app_interface.go
Comment thread translib/subscribe_app_utils.go Outdated
Comment thread translib/app_interface.go
// 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)
Copy link
Copy Markdown

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)

Copy link
Copy Markdown
Contributor Author

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.

@sachinholla sachinholla requested a review from tomek-US April 21, 2023 04:25
Copy link
Copy Markdown

@tomek-US tomek-US left a 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!

Comment thread translib/internal/apis/notify.go
Comment thread translib/internal/apis/db_diff.go
Copy link
Copy Markdown

@tomek-US tomek-US left a comment

Choose a reason for hiding this comment

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

LGTM

@tomek-US tomek-US merged commit 6ce18c6 into sonic-net:master Apr 25, 2023
@sachinholla sachinholla deleted the subscribe_app_intf branch June 18, 2023 03:41
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