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

Add SaveStartupConfig for SaveOnSet feature #83

Closed
wants to merge 16 commits into from

Conversation

rlucus
Copy link

@rlucus rlucus commented May 9, 2023

tomek-US
tomek-US previously approved these changes Jun 26, 2023
Copy link

@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.

Approving based on Sachin's approval.

@kwangsuk
Copy link
Contributor

A question -
Say, config changes are made from other apps, but 'config save' not issued yet on the device.
Meanwhile, enabling save-on-set does forcefully save all the changes including one of the last gnmi-set operation?

@rlucus
Copy link
Author

rlucus commented Oct 30, 2023

Updated CL with agreed upon method of using minimum version of rpc_copy

@rlucus
Copy link
Author

rlucus commented Nov 9, 2023

A question - Say, config changes are made from other apps, but 'config save' not issued yet on the device. Meanwhile, enabling save-on-set does forcefully save all the changes including one of the last gnmi-set operation?

Yes that would be the case.

@mbalachandar
Copy link
Contributor

Its out-of-date with base branch, please merge the latest from sonic-net:master

@rlucus
Copy link
Author

rlucus commented Nov 15, 2023

Done

Copy link
Contributor

@sachinholla sachinholla left a comment

Choose a reason for hiding this comment

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

Latest commit is introducing many rpcs and options that are not really supported. This is not a clean approach. Approach of having a translib API would have been better -- copy r s is a standard operation in NOS world.

@anand-kumar-subramanian
Copy link
Contributor

Latest commit is introducing many rpcs and options that are not really supported. This is not a clean approach. Approach of having a translib API would have been better -- copy r s is a standard operation in NOS world.

Anand>> Please have only the necessary RPC implemented end to end. Others will come in when Dell plans to bring it in.

@rlucus
Copy link
Author

rlucus commented Nov 28, 2023

Latest commit is introducing many rpcs and options that are not really supported. This is not a clean approach. Approach of having a translib API would have been better -- copy r s is a standard operation in NOS world.

Anand>> Please have only the necessary RPC implemented end to end. Others will come in when Dell plans to bring it in.

They are returning Unimplemented. Do you want them removed from the yang and annotation as well? Please be more specific, it seems as if Sachin is rejecting the whole concept of the compromise we all agreed on.

@anand-kumar-subramanian
Copy link
Contributor

Latest commit is introducing many rpcs and options that are not really supported. This is not a clean approach. Approach of having a translib API would have been better -- copy r s is a standard operation in NOS world.

Anand>> Please have only the necessary RPC implemented end to end. Others will come in when Dell plans to bring it in.

They are returning Unimplemented. Do you want them removed from the yang and annotation as well? Please be more specific, it seems as if Sachin is rejecting the whole concept of the compromise we all agreed on.

Anand>>I meant only the RPC to perform config save needs to be implemented end to end all others will be implemented when Dell plans to bring it in. Other RPCs should not be in the YANG, annotation or the transformer implementation.

@rlucus
Copy link
Author

rlucus commented Nov 30, 2023

Anand>>I meant only the RPC to perform config save needs to be implemented end to end all others will be implemented when Dell plans to bring it in. Other RPCs should not be in the YANG, annotation or the transformer implementation.

Removed the requested stubs. I'm going to have to come back to this in a few days to figure out why the linter doesn't like removing them.

@rlucus
Copy link
Author

rlucus commented Mar 27, 2024

Closing this PR as its no longer needed due to using sonic services client in sonic-net/sonic-gnmi#108

@rlucus rlucus closed this Mar 27, 2024
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.

6 participants