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

Separate out feature toggles for moniker streaming and sideband streaming #1144

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

doshirohan
Copy link
Contributor

@doshirohan doshirohan commented Dec 26, 2024

What does this Pull Request accomplish?

We had single feature toggle sideband-streaming for moniker based streaming as well as sideband streaming support for moniker based streaming. This change separates out both of these into different feature toggles.

Why should this Pull Request be merged?

Moniker based streaming that uses sideband streaming mechanism opens up unsecured raw socket for communication. To make grpc-device more secure, we want sideband streaming to be OFF by default so that additional port is not opened up for users that don't need sideband based streaming mechanism. Since moniker based streaming is behind feature toggle, I have added sideband streaming with moniker as a feature toggle as well. In future, sideband streaming using moniker could be a configuration instead of feature toggle.

What testing has been done?

Validated with local client examples that

  • Moniker streaming APIs are enabled only when moniker_streaming feature toggle is set to true.
  • Sideband streaming is enabled only when both moniker_streaming and moniker_streaming_sideband_support feature toggles are set to true.

source/server/core_server.cpp Show resolved Hide resolved
@doshirohan doshirohan merged commit f515164 into main Dec 27, 2024
9 of 10 checks passed
@doshirohan doshirohan deleted the users/indoshir/update-streaming-feature-toggles branch December 27, 2024 06:34
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