Refactor(eos_designs): Optimized the render of static routes structure config#6722
Refactor(eos_designs): Optimized the render of static routes structure config#6722MaheshGSLAB wants to merge 9 commits intoaristanetworks:develfrom
Conversation
|
Review docs on Read the Docs To test this pull request: # Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-6722
# Activate the virtual environment
source test-avd-pr-6722/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/MaheshGSLAB/avd.git@static-routes-refactor#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
python -m ansible.cli.galaxy collection install git+https://github.com/MaheshGSLAB/avd.git#/ansible_collections/arista/avd/,static-routes-refactor --force
cd test-avd-pr-6722
# Run your playbook using `python -m ansible.cli.playbook path/to/playbook.yml ...`You can also test this PR using AVD playground:
|
python-avd/pyavd/_eos_designs/structured_config/inband_management/__init__.py
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_designs/structured_config/network_services/ipv6_static_routes.py
Outdated
Show resolved
Hide resolved
python-avd/pyavd/_eos_designs/structured_config/base/__init__.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Shivani-gslab <145646625+Shivani-gslab@users.noreply.github.com>
|
|
||
|
|
||
| def append_gateway_routes( | ||
| routes_list: EosCliConfigGen.StaticRoutes | EosCliConfigGen.Ipv6StaticRoutes, |
There was a problem hiding this comment.
nayve lets add some override to prevent an IPv4 route to be added to an ipv6 list?'
|
@MaheshGSLAB we have merged #6729 which will give you access to stuctured_config inside structured_config_utils so that will simplify your PR a lot. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #6722 +/- ##
=======================================
Coverage 87.20% 87.20%
=======================================
Files 350 350
Lines 19152 19158 +6
Branches 3585 3579 -6
=======================================
+ Hits 16701 16707 +6
Misses 1887 1887
Partials 564 564
🚀 New features to boost your workflow:
|
|
| self.set_static_routes( | ||
| "ipv4", | ||
| self.shared_utils.mgmt_gateway, | ||
| self.inputs.mgmt_interface_vrf, |
There was a problem hiding this comment.
I think this is not needed to move here as it is called only once.
| @@ -112,31 +112,12 @@ def router_bgp(self) -> None: | |||
| @structured_config_contributor | |||
| def static_routes(self) -> None: | |||
There was a problem hiding this comment.
@MaheshGSLAB I think we discussed that this was probably not the best to have it splitted here and then called twice and then calling a single helper that needs to discriminate between ipv4 and ipv6 routes
I am not sure there is any gain in what we are doing here for the management IP
gmuloc
left a comment
gmuloc
left a comment
There was a problem hiding this comment.
Looking at this PR - this is not bringing any value towards refactoring the codebase but making it more complex.
Logic should not moved to structured_config_utils like this
lets discuss this over a call.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
5 participants
Change Summary
Optimized the render of static routes structure config
Related Issue(s)
Fixes #
Component(s) name
arista.avd.eos_designsProposed changes
Add helper method and run_once decorator to optimize the render of static_routes structure config
How to test
CI should be green without structure config and cfg changes
Checklist
User Checklist
Repository Checklist