Skip to content

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

Merged
merged 5 commits into from
Sep 22, 2021
Merged

AML-4 Algo CLI Auth issue #111

merged 5 commits into from
Sep 22, 2021

Conversation

zeryx
Copy link
Contributor

@zeryx zeryx commented Sep 20, 2021

It was discovered that the algo auth command 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.

@zeryx zeryx self-assigned this Sep 20, 2021
@@ -354,14 +358,18 @@ def getAPIkey(self,profile):
config_dict = toml.load(key)
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'] != "":
Copy link
Contributor

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 != "")
Copy link
Contributor

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
kennydaniel previously approved these changes Sep 21, 2021
Copy link
Contributor

@kennydaniel kennydaniel left a 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.

@zeryx zeryx merged commit 2915fc8 into develop Sep 22, 2021
@zeryx zeryx deleted the AML-4-CLI-auth-issue branch September 22, 2021 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants