-
Notifications
You must be signed in to change notification settings - Fork 4
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 done while validating solution for LGI Polaris project #110
base: develop
Are you sure you want to change the base?
Conversation
katzy687
commented
Aug 25, 2021
- order of populating connection method - need network_cli defined by user var, shell overrides everything
- single quote wrapping of vars for passing JSON. Need a single quote on outside instead of double quotes. single quotes still valid regular strings
-
- SSL evaluation fix - why evaluate the boolean based on data sent by server? this couples the shell to cloudshell version. The setting can be read from context.attributes and shell can work on older versions of cloudshell as is now
- Made the password flow more agnostic - if token or password is populated flow will still work. Makes driver backwards compatible for users on versions without token field yet.
…SSL Verification read from context attribute, single quoting vars for JSON vars
:type ansi_conf: AnsibleConfiguration | ||
:type cancellation_sampler: CancellationSampler | ||
:type logger: Logger | ||
:rtype str | ||
""" | ||
repo = ansi_conf.playbook_repo | ||
auth = None | ||
if ansi_conf.playbook_repo.username or ansi_conf.playbook_repo.token: | ||
if ansi_conf.playbook_repo.username or ansi_conf.playbook_repo.token or ansi_conf.playbook_repo.password: |
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.
what is purpose here?
can there be a password if no username?
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.
there can be a token with no username. My use case here was backwards support of driver for users on system without the token attribute field available yet. If the token field is empty fallback to the password. Customers can use the driver and won't have to migrate attributes from password to token.
@@ -33,7 +33,8 @@ def __exit__(self, type, value, traceback): | |||
with self.file_system.create_file(self.file_path) as file_stream: | |||
lines = ['---'] | |||
for key, value in sorted(self.vars.items()): | |||
lines.append(str(key) + ': "' + str(value) + '"') | |||
# lines.append(str(key) + ": '" + str(value) + "'") |
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.
prefer to delete than to comment
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.
sure
@@ -67,10 +67,11 @@ def _download(self, url, auth, logger, cancel_sampler, verify_certificate): | |||
if not response_valid and auth is None: | |||
raise Exception('Please make sure the URL is valid, and the credentials are correct and necessary.') | |||
|
|||
generic_auth = auth.token if auth.token else auth.password |
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.
there is a different section intended to handle auth password further down, if the normal token path does not succeed.
What is the intention of using this path for auth password as well?
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.
for supporting older customers that can't upgrade yet to also use tokens
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.
So some of the questions might be a bit naive, but I just want to understand your intentions here.
Also not sure I understood everything you wrote in your comment, especially the part of "order of populating connection method - need network_cli defined by user var, shell overrides everything"