-
Notifications
You must be signed in to change notification settings - Fork 99
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
"wsk property get" should return raw output when provided with specific property. #430
Conversation
Fixing integration tests as they depend on output "text" of 'wsk property get' command. |
@neerajmangal - currently, this appears to be a breaking change.. that is, existing scripts will fail when this is merged as-is. what about adding a new flag - that could be used by all commands - to specify the format of the output (i.e. json, raw, etc) |
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 awesome! Thank you for implementing this.
Hi @mdeuser , Yes this will possibly break existing scripts if they are using "property get" and then parsing the output. I initially thought of adding a "--raw" option but let me know what should be the best option in this case. But I believe "-- propertyname" can be think of raw input to the command "wsk property get".
|
what about a
additional output formats can be added without requiring new option flags. |
@mdeuser , I attempted an implementation as suggested. New option --output|-o is added to property get command for now with std|raw.
default behavior is "std" if no --output|-o option is provided
With raw Option
If "raw" is provided with --all or without property it will return an error.
check for bugus output type.
if "std" provided with --all or without a specific property, cmd will treat it as default behavior.
I am writing test cases for new output flag and also will update the documentation. |
fantastic work 👏 @neerajmangal - thanks for these contributions. |
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.
Looking good!
this behavior is perfect @neerajmangal ! 💯 🥇 thanks!! |
only item for discussion could be the use of |
Hi @mdeuser, I think we should get cut-off default|std output type itself. I mean, if --ouput|-o is provided then it will be a formatted output as raw|json|yaml otherwise it will throw an error. If --output|-o not provided then cmd will display as of today. Let me know your suggestions. |
Hi @mdeuser, please let me know your thoughts on above. I should modify "std" flag further or it should be good for now? Thanks, |
@neerajmangal - i'm not sure i entirely understand the following
imho, all supported output format types should be able to be specified via the |
Is this ready to merge now? |
std is ok with me - will wait for Mark in case he feels strongly. |
@neerajmangal do you mind sending an email to the Apache dev list about this PR - just a short description of what you've done and a link to the PR. |
using |
Thanks @neerajmangal for this contribution. |
This PR attempts to address issues explained in #407.
With this PR, wsk CLI will provide a raw output when a specific property is provided in "wsk property get" command.
"wsk property get"
If Option "--all" is present with other options then --all takes precedence.