Feature/add supported infra to samples #12
Merged
Conversation
This file contains hidden or 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
There was a problem hiding this comment.
Pull Request Overview
This PR adds infrastructure validation support, refactors resource cleanup logic, and updates enum usage across the codebase.
- Introduced a
validate_infrastructurefunction with accompanying tests - Refactored and de-duplicated
_cleanup_resourcesimplementation - Migrated
Enumsubclasses toStrEnum - Injected infrastructure validation into sample notebooks
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/python/test_utils.py | Added imports and tests for validate_infrastructure |
| shared/python/utils.py | Introduced _cleanup_resources, removed duplicate, and implemented validate_infrastructure |
| shared/python/apimtypes.py | Converted Enum subclasses to StrEnum |
| samples/load-balancing/create.ipynb | Added supported_infrastructures list and validation call |
| samples/general/create.ipynb | Added supported_infrastructures list and validation call |
| samples/_TEMPLATE/create.ipynb | Provided template slot for supported_infrastructures and validation call |
Comments suppressed due to low confidence (1)
shared/python/utils.py:185
- The code calls
traceback.print_exc()but there is noimport tracebackin this module, which will raise aNameError. Addimport tracebackat the top.
traceback.print_exc()
There was a problem hiding this comment.
Pull Request Overview
This PR adds infrastructure support to the samples by introducing new tests and functions for validating supported infrastructures. Key changes include:
- Adding tests in tests/python/test_utils.py to verify the new validate_infrastructure functionality.
- Introducing a new _cleanup_resources helper and a proper validate_infrastructure function in shared/python/utils.py.
- Updating enums in shared/python/apimtypes.py to use StrEnum.
- Updating sample notebooks to incorporate and demonstrate the new supported infrastructures pattern.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/python/test_utils.py | Added tests for the new validate_infrastructure function |
| shared/python/utils.py | Added _cleanup_resources and validate_infrastructure, and removed duplicate validation function usage |
| shared/python/apimtypes.py | Updated enum definitions to use StrEnum |
| samples/load-balancing/create.ipynb | Added supported_infrastructures list and validate_infrastructure usage |
| samples/general/create.ipynb | Added supported_infrastructures list and validate_infrastructure usage |
| samples/_TEMPLATE/create.ipynb | Added placeholder for supported_infrastructures and validation call |
Comments suppressed due to low confidence (1)
shared/python/utils.py:183
- The _cleanup_resources function catches all exceptions and then calls traceback.print_exc(), but there is no import statement for the traceback module. Consider importing traceback or removing its usage.
except Exception as e:
| # PRIVATE METHODS | ||
| # ------------------------------ | ||
|
|
||
| def _cleanup_resources(deployment_name: str, rg_name: str) -> None: |
There was a problem hiding this comment.
The function docstring for _cleanup_resources indicates that it raises an Exception on error, but the implementation catches and logs exceptions instead. Update the docstring to reflect the actual behavior.
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.
Resolves #11