-
Notifications
You must be signed in to change notification settings - Fork 38
AML-4 Algo CLI Auth issue #111
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
Conversation
Algorithmia/CLI.py
Outdated
| apikey = None | ||
| if('profiles' in config_dict.keys() and profile in config_dict['profiles'].keys()): | ||
| apikey = config_dict['profiles'][profile]['api_key'] | ||
| if config_dict['profiles'][profile]['api_key'] != "": |
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 could be an and clause in the previous if.
| client = Algorithmia.client(CLI().getAPIkey(profile), CLI().getAPIaddress(profile), CLI().getCert(profile)) | ||
| result2 = CLI().ls("data://.my", client) | ||
|
|
||
| self.assertTrue(result2 != "") |
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.
Be more specific about checking the result (not empty is a pretty loose test)
kennydaniel
left a 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.
Some nits, overall looks fine.
It was discovered that the
algo authcommand bundled into the CLI will fail if the API Address is any cluster other than the Marketplace. This was isolated to line 34 of the CLI.py file and a test was added to (hopefully) ensure we catch issues like that in the future.