-
Notifications
You must be signed in to change notification settings - Fork 393
T7915: minor fixes for consistent exception handling and error messages #4814
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
Conversation
|
👍 |
|
Move to draft to fix overly-restrictive treatment of Useless_set as error: this has been seen in automated tests --- need to allow redundant set operations. |
817482e to
12975ed
Compare
|
Added: expose Note that this has now a hard requirement on vyos/vyos1x-config#54 so CI integration is not possible until that is merged. |
|
Rebased over current and resolved conflicts. |
|
CI integration 👍 passed! Details
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request improves error handling in the VyOS configuration tree library by enhancing error messages and adding better exception handling. The changes make error reporting more consistent and informative throughout the library.
Key Changes:
- Added
value_exists()function to check if a specific value exists at a path - Enhanced error handling to retrieve and display detailed error messages from the underlying OCaml library
- Improved return value checking for
is_leaf()to properly return boolean values - Updated dependency versions for vyos1x-config and vyconf libraries
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/vyos/configtree.py | Added value_exists() method and improved error handling with detailed messages from underlying library |
| libvyosconfig/lib/bindings.ml | Enhanced exception handling with proper error message setting and alert annotations for all exceptions |
| libvyosconfig/Makefile | Updated dependency commit hashes for vyos1x-config and vyconf libraries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The addition of alert exn to vyos1x-config allows an audit of all raised exceptions, providing fixes for consistent handling and error reporting.
|
CI integration ❌ failed! Details
|
dmbaturin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot find any objections against this logic.
c-po
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update hash and fix to warnings.
I trust @jestabro on this.
Also adds nice value_Exists utility function
Change summary
NOTE: this requires
vyos/vyos1x-config#54
vyos/vyconf#33
and an update to this PR to point to those repos after merge. Integration tests will not run until the first of those is merged and pointed to.
<-- prerequisites merged and hash references updated
The addition of alert exn to vyos1x-config allows an audit of all raised exceptions, providing fixes for consistent handling and error reporting.
Types of changes
Related Task(s)
Related PR(s)
vyos/vyos1x-config#54
vyos/vyconf#33
How to test / Smoketest result
Checklist: