Python: Refactor environment variable naming for clarity and consistency #5686
Conversation
This file contains 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
- Updated environment variable names to use single underscores for better readability and to avoid potential confusion. - Modified `service_configurator.py` to align with the updated environment variable naming convention. - Updated `.env.example` to reflect the new environment variable names, ensuring consistency across the project. - Added error handling and improved error messages in `service_configurator.py` for better debugging and maintainability.
markwallace-microsoft
added
python
github-actions
bot
changed the title
|
unit test |
Unit test indeed all runs, lint does not, please run the checks! |
Python 3.8 Test Coverage Report •
Python 3.8 Unit Test Overview
|
moonbox3
approved these changes
Apr 3, 2024
moonbox3
approved these changes
Apr 3, 2024
|
HI, I've noticed that this PR hasn't been approved yet. Are there any issues, or is there anything else I should add? Please let me know :) |
Thanks for following up. Just need one more approval from @eavanvalkenburg or @juliomenendez. |
juliomenendez
approved these changes
Apr 16, 2024
|
Thank you for your contribution, @bx-h. |
LudoCorporateShark
pushed a commit
to LudoCorporateShark/semantic-kernel
that referenced
this pull request
Aug 25, 2024
…ncy (microsoft#5686) ### Summary of Changes - Updated environment variable names to use single underscores for better readability and to avoid potential confusion. - Modified `service_configurator.py` to align with the updated environment variable naming convention. - Updated `.env.example` to reflect the new environment variable names, ensuring consistency across the project. - Added error handling and improved error messages in `service_configurator.py` for better debugging and maintainability. ### Motivation and Context The motivation for these changes is to address a bug where the `service_configurator.py` script is unable to read the `GLOBAL_LLM_SERVICE` environment variable due to inconsistent naming conventions. Additionally, the Azure service configuration variables were not being utilized, which could potentially lead to misconfigurations in certain deployment scenarios. - **Why is this change required?** To fix existing bugs related to environment variable handling and improve code maintainability and readability. - **What problem does it solve?** Ensures reliable loading of environment variables and correct configuration of AI services, thereby preventing runtime errors. - **What scenario does it contribute to?** Enhances the stability and reliability of the AI service configuration process within the semantic-kernel project. ### Description This PR addresses the issue where the environment variable `GLOBAL_LLM_SERVICE` was not consistently read due to a mismatch in the expected naming convention. The problem was identified in the `service_configurator.py` script where the variable was referenced with two underscores instead of one. Additionally, Azure service deployment names were not being used, leading to potential misconfigurations. - Modified the naming convention from double to single underscores for all environment variables in `service_configurator.py` and `.env.example`. - Ensured that Azure service deployment names are correctly utilized in the service configuration logic. - Improved error handling in the script to provide clearer and more actionable feedback. For more context, here's a link to the buggy code: [Link to the code](https://github.com/microsoft/semantic-kernel/blob/main/python/samples/documentation_examples/service_configurator.py#L27), [Link to the code](https://github.com/microsoft/semantic-kernel/blob/main/python/samples/documentation_examples/.env.example#L1)   ### Contribution Checklist - [x] The code builds clean without any errors or warnings. - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations. - [x] All unit tests pass, and I have added new tests where possible. - [x] I didn't break anyone 😄 --------- Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com> Co-authored-by: Chris <66376200+crickman@users.noreply.github.com>
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.
Summary of Changes
service_configurator.pyto align with the updated environment variable naming convention..env.exampleto reflect the new environment variable names, ensuring consistency across the project.service_configurator.pyfor better debugging and maintainability.Motivation and Context
The motivation for these changes is to address a bug where the
service_configurator.pyscript is unable to read theGLOBAL_LLM_SERVICEenvironment variable due to inconsistent naming conventions. Additionally, the Azure service configuration variables were not being utilized, which could potentially lead to misconfigurations in certain deployment scenarios.Description
This PR addresses the issue where the environment variable
GLOBAL_LLM_SERVICEwas not consistently read due to a mismatch in the expected naming convention. The problem was identified in theservice_configurator.pyscript where the variable was referenced with two underscores instead of one. Additionally, Azure service deployment names were not being used, leading to potential misconfigurations.service_configurator.pyand.env.example.For more context, here's a link to the buggy code: Link to the code, Link to the code
Contribution Checklist