Skip to content
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

BGP AF Parsing Optimization #688

Closed

Conversation

bentole
Copy link
Contributor

@bentole bentole commented Nov 14, 2022

SUMMARY

This is a supplement to PR #687 by optimizing parsing of existing Jinja2 templates.

It improves the time it takes to deploy playbooks to routers with massive BGP configurations as discussed in #682 avoiding socket path /home/bol/.ansible/pc/9857e85642 does not exist or cannot be found

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cisco.ios.ios_address_family

ADDITIONAL INFORMATION

To verify the effects of this PR, use the playbook and the configuration mentioned: here.

I have also created a test case for this issue here which demonstrates the differences between the optimized vs. original parsers.

@bentole bentole marked this pull request as ready for review November 14, 2022 10:20
@bentole bentole changed the title Optimize BGP AF Parsing BGP AF Parsing Optimization Nov 14, 2022
@KB-perByte
Copy link
Collaborator

Hey @bentole looking at your fix it would make it overly complicated on basis of the parsing that is being done.
please check this PR #687
you may pull the PR and try out a parsed state playbook that just crunches the facts for you.
I am working on the config side code. Please expect the PR to be ready in a day or two.
Regards

@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 56s
✔️ build-ansible-collection SUCCESS in 3m 53s
ansible-ee-integration-ios-latest NODE_FAILURE in 0s (non-voting)
ansible-ee-integration-ios-stable-2.9 FAILURE in 15m 40s (non-voting)
ansible-ee-integration-ios-stable-2.11 FAILURE in 14m 48s (non-voting)
ansible-ee-integration-ios-stable-2.12 FAILURE in 15m 28s (non-voting)
ansible-ee-integration-ios-libssh-latest NODE_FAILURE in 0s (non-voting)
ansible-ee-integration-ios-libssh-stable-2.9 FAILURE in 41m 04s (non-voting)
ansible-ee-integration-ios-libssh-stable-2.11 FAILURE in 43m 57s (non-voting)
ansible-ee-integration-ios-libssh-stable-2.12 FAILURE in 42m 48s (non-voting)
✔️ ansible-tox-linters SUCCESS in 5m 05s

@bentole
Copy link
Contributor Author

bentole commented Nov 14, 2022

Hey @bentole looking at your fix it would make it overly complicated on basis of the parsing that is being done.
please check this PR #687
you may pull the PR and try out a parsed state playbook that just crunches the facts for you.
I am working on the config side code. Please expect the PR to be ready in a day or two.
Regards

Sounds good! Didnt see your comment at first so wrote you a reply here 😃

@bentole
Copy link
Contributor Author

bentole commented Nov 14, 2022

Hey @bentole looking at your fix it would make it overly complicated on basis of the parsing that is being done.

Forgot to comment on this :) It appears complicated since I'm overriding functions from NetworkTemplate and that requires a copy of the complete method, but the only lines really added are:

lookup_keys = capdict.keys()

for idx, entry in enumerate(tval):
  tval.pop(idx)
  _entry = deepcopy(entry)
  for k, v in _entry.items():
      if k not in data.keys() and k in lookup_keys:
          entry.pop(k)
  tval.append(entry)

But again a solution that does not involve overriding methods is definately preferable.

@bentole bentole closed this Nov 16, 2022
@bentole
Copy link
Contributor Author

bentole commented Nov 16, 2022

The issue that this PR addressed is covered by and fixed more extensively in PR #687

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants