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

feat(anta.tests)!: Implement caching #394

Merged
merged 18 commits into from
Sep 28, 2023

Conversation

carl-baillargeon
Copy link
Collaborator

@carl-baillargeon carl-baillargeon commented Sep 14, 2023

Description

Implement caching with aiocache library.

Cache key design:

<device_name>:<uid>

The uid is a new property attribute of AntaCommand, which is a UID generated from the command, version, revision and output format.

Each UID has its own asyncio Lock. This way, coroutines that need to access the cache for different uid's can do so concurrently.

Caching is implemented in the AntaDevice.collect_commands async function where the inner collect_command function first checks if the command.uid is present in the cache and if it is, command.output will be populate by the cache. If not, the collect() function will be run as usual and the output will be saved in the cache.

Fixes #342

Tasks:

  • Add a knob in AntaTest to activate/deactivate caching for a specific test
  • Add logging with aiocache plugins for cache hit ratio, etc.
  • Implement click CLI options to tweak caching (TTL, etc.)
  • Maybe need to add a coroutine to check and clean-up unused Locks.
  • Add functionalities to support external backend caches like Redis/memcached - future PR.
  • Documentation + examples

Caveats:

  • Some tests in ANTA will break with caching. Logging tests are a good example where we generate a log message and check for that log right after. That's why we need a way to activate/deactivate caching for specific tests.
  • Since there is a lot of varied UIDs and each device has its own instance of the AntaDevice class, we might need a lock cleanup strategy to optimize performance when running thousands of tests on hundreds devices.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have run pre-commit for code linting and typing (pre-commit run)
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (tox -e testenv)

anta/device.py Outdated Show resolved Hide resolved
anta/device.py Outdated Show resolved Hide resolved
@titom73 titom73 added this to the v0.9.0 milestone Sep 22, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Collaborator

@gmuloc gmuloc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Let's add a way in the inventory to disable the cache per device if needed
  • Let's add a CLI to disable the cache completely as well (as mentioned in your PR)
  • Documentation to add as well with examples maybe
    • Disabling cache for a command
    • Disabling cache globally
    • Disabling cache for a device from the inventory

anta/models.py Outdated Show resolved Hide resolved
anta/device.py Outdated Show resolved Hide resolved
anta/device.py Show resolved Hide resolved
@carl-baillargeon carl-baillargeon marked this pull request as ready for review September 28, 2023 02:59
docs/README.md Outdated Show resolved Hide resolved
docs/advanced_usages/caching.md Outdated Show resolved Hide resolved
tests/units/test_device.py Outdated Show resolved Hide resolved
@titom73 titom73 self-requested a review September 28, 2023 10:11
Copy link
Collaborator

@titom73 titom73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good after resolving points raised by @gmuloc

@gmuloc gmuloc merged commit 03a0ede into arista-netdevops-community:main Sep 28, 2023
17 checks passed
@carl-baillargeon carl-baillargeon deleted the feat/cache branch May 18, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement caching for AntaDevice.collect()
4 participants