Resolves idempotency issue in future() #166
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
jtdub
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces structural idempotency matching for the future() method to correctly distinguish between commands that share common prefixes but have different structural identities (like BGP neighbor configurations with different IP addresses). The changes include new idempotency key generation logic, a comprehensive regression test, and documentation updates for the new feature and Fortinet FortiOS driver support.
- Enhanced idempotency matching algorithm using structural keys based on match rules
- Added regression test for BGP neighbor description preservation
- Documented structural idempotency matching feature (version 3.3.1)
- Added Fortinet FortiOS to the list of supported platforms
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/test_hier_config.py | Adds regression test verifying BGP neighbor descriptions are correctly preserved during future/rollback operations |
| hier_config/platforms/driver_base.py | Implements structural idempotency key generation with helper methods for matching prefixes, suffixes, contains, and regex patterns |
| docs/future-config.md | Documents the new structural idempotency matching feature introduced in version 3.3.1 |
| docs/drivers.md | Adds Fortinet FortiOS driver documentation and formatting improvements throughout the driver documentation sections |
| docs/index.md | Adds Fortinet FortiOS to the supported platforms list |
| README.md | Adds Fortinet FortiOS to supported platforms, new Highlights section showcasing the improved future() functionality, and formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Increase driver_base.py coverage from 78% to 98% - Add 25 new tests covering all idempotency key code paths - Test all match rule types: equals, startswith, endswith, contains, regex - Test edge cases: negated commands, lineage mismatch, tuple matching - Test regex features: capture groups, greedy patterns, normalization - Verify issue #157 fix with comprehensive test scenarios All tests pass (74 in test_hier_config.py, 118 total)
- Move all imports to top-level (fix PLC0415) - Add noqa: SLF001 for intentional private member access - Sort imports properly (fix I001) All tests passing, ruff checks clean.
- Add module docstring and pylint disable for too-many-lines - Add pyright ignore comments for intentional private member access (25 instances) - Fix pylint C1803: Change 'key == ()' to 'not key' for implicit boolean check All tests passing, pyright 0 errors, ruff checks clean.
…ptions-diaeV Fix BGP neighbor descriptions dropped in future config
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.
closes #157