Conversation
prmukherj
changed the title
prmukherj
requested review from
hpohekar,
mayankansys and
seanpearsonuk
as code owners
There was a problem hiding this comment.
Pull request overview
This PR improves the developer/user-facing error behavior when resolving a Fluent executable path from an AWP_ROOTnnn environment variable by replacing a low-level KeyError with a clearer FileNotFoundError, and adds a regression test to cover the missing-env-var case.
Changes:
- Raise a
FileNotFoundErrorwith a descriptive message whenFluentVersion.get_fluent_exe_path()is called and the correspondingAWP_ROOTnnnenvironment variable is not set. - Add a unit test verifying the new exception type/message behavior.
- Add a maintenance changelog entry (currently phrased inconsistently with the actual change).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ansys/fluent/core/utils/fluent_version.py |
Switches env var lookup from os.environ[...] to os.getenv(...) and raises FileNotFoundError when missing. |
tests/test_fluent_version.py |
Adds coverage for the missing AWP_ROOTnnn env var case. |
doc/changelog.d/5012.maintenance.md |
Documents the change (but currently references the wrong API/behavior). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Looks good. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
prmukherj
deleted the
maint/update_error_handling_for_get_fluent_exe_path
branch
Context
There was a low level key error raised from this method that was percolating elsewhere to the user of this code.
Change Summary
Properly documented "FileNotFoundError" is raised now.
Impact
It gives user of this method a proper error when the particular Fluent installation is not found. Test has been added for the same.