Setup build pipeline to run online tests - #106
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ensitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…oft/typeagent-py into dev/robgruen/online_tests
Guido van Rossum (gvanrossum)
left a comment
Guido van Rossum (gvanrossum)
left a comment
There was a problem hiding this comment.
This looks like an excellent addition!
Not quite a full review, I made it to line 350 or so on my phone. Mostly style nits and mumbling about exception discipline.
GitHub has some quibbles where it thinks you're writing secrets. Not sure how to satisfy it; it's pretty distracting.
| dependencies = [ | ||
| "azure-identity>=1.22.0", | ||
| "azure-mgmt-authorization>=4.0.0", | ||
| "azure-mgmt-keyvault>=12.1.1", |
There was a problem hiding this comment.
Maybe these two belong in the extras section? They aren't needed for people who just install the package.
| @@ -0,0 +1,724 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Filename should use Python convention, get_keys.py, and ditto for config file.
| import uuid | ||
| from pathlib import Path | ||
| from typing import Dict, List, Tuple, Optional, Set | ||
| from datetime import datetime |
There was a problem hiding this comment.
Alphabetize imports.
| from azure.mgmt.authorization import AuthorizationManagementClient | ||
|
|
||
|
|
||
| # ANSI color codes for terminal output |
There was a problem hiding this comment.
Use colorama instead -- already used elsewhere.
| ) | ||
| ) | ||
| except Exception as e: | ||
| print(e) |
There was a problem hiding this comment.
I'd say print(repr(e)) so the exception class names shown too.
| Colors.RED, | ||
| ) | ||
| ) | ||
| raise Exception(f"Unable to find the role '{role_name}'.") |
There was a problem hiding this comment.
Use a more specific exception name, e.g. RuntimeError.
| account_details = json.loads(result.stdout) | ||
| return account_details["id"] | ||
| except Exception as e: | ||
| print(e) |
There was a problem hiding this comment.
Again
| Colors.RED, | ||
| ) | ||
| ) | ||
| sys.exit(12) |
There was a problem hiding this comment.
What does 12 mean? Normally we use 1.
| await asyncio.sleep(5) | ||
|
|
||
| return client.get_secrets(vault_name) | ||
| except Exception: |
There was a problem hiding this comment.
Not printing the exception at all?
| print(colored("Elevation successful.", Colors.GREEN)) | ||
| print(colored("Waiting 5 seconds...", Colors.YELLOW)) | ||
| await asyncio.sleep(5) | ||
| except Exception: |
There was a problem hiding this comment.
Again
3 participants
Uh oh!
There was an error while loading. Please reload this page.