-
Notifications
You must be signed in to change notification settings - Fork 25
Add cli flag for initial state response output in events #674
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
base: main
Are you sure you want to change the base?
Conversation
| Status: approved | ||
|
|
||
| The Ankaios CLI shall provide a function to get real-time state change events from the Ankaios server with a field mask as argument for filtering events to specific parts of the state. | ||
| The Ankaios CLI shall provide a function to get real-time state change events from the Ankaios server with the following arguments: |
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.
Shall we increase the swdd id of this requirement?
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.
As we not only made the requirement more specific, but changed how we understand it (the part with the details is added), I would increase the SwDD ID.
| #[arg(short = 'o', value_enum, default_value_t = OutputFormat::Yaml)] | ||
| output_format: OutputFormat, | ||
| /// Specify if the first complete state response shall be included in the output | ||
| #[arg(short = 'd', long = "detailed", default_value_t = false)] |
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.
We can discuss about a better argument name with pleasure. I tried to avoid introducing a flag for already known intention.
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.
Maybe "--include-current" and "-c"? I think it describes the argument better. The name "detailed" implies it return more information all the time, not only at the beginning.
| # [stest->swdd~cli-provides-get-events-command~1] | ||
| Test Ankaios CLI get events with initial complete state output | ||
| [Documentation] Subscribe to events and output initial complete state response | ||
| [Setup] Setup Ankaios | ||
|
|
||
| # Preconditions | ||
| Given Podman has deleted all existing containers | ||
| And Ankaios server is started with manifest "${CONFIGS_DIR}/nginx.yaml" | ||
| And Ankaios server is available | ||
| And Ankaios agent is started with name "agent_A" | ||
| # Actions | ||
| When user starts the CLI to subscribe to events with detailed mode enabled with format "yaml" and field mask "desiredState.workloads" in background | ||
| And the user waits "1" seconds | ||
| # Asserts | ||
| Then the event output shall contain workload "nginx" | ||
| And the event output shall be valid yaml format | ||
| And the event output shall contain only desiredState workloads | ||
| [Teardown] Clean up Ankaios |
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.
I think it would be better if this test is more similar to "Test Ankaios CLI get events with field mask filter", updating the state after subscribing to the events. Then we could test if getting the initial state and getting events work on the same time.
| Status: approved | ||
|
|
||
| The Ankaios CLI shall provide a function to get real-time state change events from the Ankaios server with a field mask as argument for filtering events to specific parts of the state. | ||
| The Ankaios CLI shall provide a function to get real-time state change events from the Ankaios server with the following arguments: |
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.
As we not only made the requirement more specific, but changed how we understand it (the part with the details is added), I would increase the SwDD ID.
| #[arg(short = 'o', value_enum, default_value_t = OutputFormat::Yaml)] | ||
| output_format: OutputFormat, | ||
| /// Specify if the first complete state response shall be included in the output | ||
| #[arg(short = 'd', long = "detailed", default_value_t = false)] |
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.
Maybe "--include-current" and "-c"? I think it describes the argument better. The name "detailed" implies it return more information all the time, not only at the beginning.
Issues: #450
The PR introduces the following changes:
--detailed(-d) event CLI flag for the cli to enable the output of the initial state response when subscribing to eventsreceive_next_eventfunctionget_complete_state)Definition of Done
The PR shall be merged only if all items mentioned in CONTRIBUTING.md have been followed. In case an item is not applicable as described, please provide a short explanation in the description.