-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fixes #335 #338: Modify so general opts work in interactive mode #336
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since PR #306 is now merged, please rebase this on master. |
KSchopmeyer
force-pushed
the
ks/#335/use-general-opts
branch
2 times, most recently
from
September 14, 2019 20:44
b144bdb
to
cdae6e0
Compare
KSchopmeyer
force-pushed
the
ks/#335/use-general-opts
branch
4 times, most recently
from
September 14, 2019 22:31
87f2c58
to
bbe4798
Compare
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
reviewed
Sep 15, 2019
andy-maier
requested changes
Sep 15, 2019
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.
See comments
KSchopmeyer
force-pushed
the
ks/#335/use-general-opts
branch
2 times, most recently
from
September 15, 2019 19:37
5706e01
to
c0c202a
Compare
KSchopmeyer
changed the title
WIP Ks/#335/use general opts
Fixes issues # 335 and # 338, Modify so general opts work in interactive mode.
Sep 15, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
reviewed
Sep 16, 2019
andy-maier
requested changes
Sep 16, 2019
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.
Some more comments
andy-maier
reviewed
Sep 16, 2019
…ions. Modified the structure to allow use of the general options within an interactive session. 1. Removed some of the properties (timestats, use_pull, pull_max_cnt, log, verbose) from PywbemServer class. These are maintained only in the context and therefore, when set, they apply to all commands, not any particular pywbem server. They were always accessed from the context rather than the server so should never have been in the server. 2. Added property setters to PywbemServer class so can individually set properties. 3. Modified no_verify option. Modified no-verify option to allow 3 values (None, True, False). This allows the code to determine if the option was defined. 4. Reorganized pywbemcli.py slightly to do function cli input argument processing on all calls, startup and command calls. This produces resolve_... versions of each of the input arguments so that the code can determine if the argument was actually input or the default was set. 5. Changed the name of the no_verify option to verify because the order of processing in the click option that handles '--a/--no_a' is such that we would have either had to define it as '--no-a/--a' or invert the argument in processing. Besides this actually seems more logical instead of the double negative (i.e. is no-verify True or false). Note that the alternative would have been to make it a choice argument (choice ("true", "false"). We had to do something so that we could determine if the argument was input or not and the is-flag does not do that. 6. Remove the block on overwrite of connection definitions with 'connection save'.
change to code to simplify, and . Does not include change for proposed move of use_pull and pull_max_cnt back to the connection defintion or the change for interactive mode on connection show.
help and also expands the _show command to more explicitly define why ic cannot show anything (no connections file, no current connection, etc.) The messages now show where the connections file should be if it is empty/missing. Added more tests for the errors with empty repo on connection show, delete, select.
KSchopmeyer
force-pushed
the
ks/#335/use-general-opts
branch
from
September 16, 2019 16:54
f46be85
to
da83ade
Compare
andy-maier
approved these changes
Sep 16, 2019
andy-maier
changed the title
Fixes issues # 335 and # 338, Modify so general opts work in interactive mode.
Fixes #335 #338: Modify so general opts work in interactive mode
Sep 16, 2019
Approved |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview:
Connection save changed to allow overwrite of the connection if already in the connections file by removing the test for existing name in the file. Since the new general options are not retained after the connection save command, there is no impact on the current connection. # issue "connection save" should overwrite existing connection definitions #338
Uses general options before all commands except connect [select]. Sets the general options values into the current connection but restores any previous connection definition for subsequent commands in the interactive session. Connection select, makes the defined option the current option for the defined interactive session but DOES not apply the modifications from the general options because they would have been applied to the current connection, not the connection we are selecting. NOTE: we do not notify the user of this issue.
Connection select with general options gets the connection from the repo, makes the changes from the general options and also makes this the current option for this interactive session. There will be NO way to select the current connect and change it, only persistent connections.
Issues:
Some options cannot be unset. Thus, you can set value into --user but you cannot set it back to None because it is the existence of the general option that causes it to be set. This applies to --user, --password, --keyfile, etc.
**COMMENT" Since click does not support the concept of options with optional arguments, see click issue: pallets/click#549, we propose that setting the value of these options to "" is a valid way to let pywbemcli set the values to None. I propose that this be a separate PR just to keep the changes to this PR limited.
Modified -no-verify per the commit message so it can be set, unset, etc. The new option is `--verify/--no-verify' and there is no short form