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

Refactor(plugins): Improve schema models #4795

Conversation

ClausHolbechArista
Copy link
Contributor

  • Add a few extra methods to schema classes to improve code readability on future refactored code.
    • provide ways of updating or adding new items inline, without having to give the full class path.
    • ._update on AvdModel and append_new on AvdList and AvdIndexedList gives type hints similar to __init__() of the item class. This means it is easy to provide the correct supported fields.
    • The new methods return the instance to make it easier to chain with other methods.

Copy link

github-actions bot commented Dec 9, 2024

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-4795
# Activate the virtual environment
source test-avd-pr-4795/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/ClausHolbechArista/avd.git@refactor/plugins/improve-schema-models#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/ClausHolbechArista/avd.git#/ansible_collections/arista/avd/,refactor/plugins/improve-schema-models --force
# Optional: Install AVD examples
cd test-avd-pr-4795
ansible-playbook arista.avd.install_examples

gmuloc
gmuloc previously approved these changes Dec 10, 2024
@gmuloc gmuloc added the one approval This PR has one approval and is only missing one more. label Dec 10, 2024
@gmuloc gmuloc requested a review from a team December 10, 2024 13:46
Copy link
Contributor

@emilarista emilarista left a comment

Choose a reason for hiding this comment

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

Tested, LGTM!

@ClausHolbechArista ClausHolbechArista dismissed gmuloc’s stale review December 11, 2024 19:15

I pushed some extra changes. Somehow it messed up so I will dismiss your review.

Copy link
Contributor

@emilarista emilarista left a comment

Choose a reason for hiding this comment

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

Retested with same repos, still LGTM!

@ClausHolbechArista ClausHolbechArista force-pushed the refactor/plugins/improve-schema-models branch from aff1d0c to 332fea6 Compare December 13, 2024 14:25
Copy link
Contributor

@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.

In general I feel like it would make sense to add some unittest for merging and inheriting with the flags for some classes to make sure we don't break this in further refactoring

python-avd/pyavd/_schema/models/avd_indexed_list.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_list.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_model.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_model.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_list.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_indexed_list.py Outdated Show resolved Hide resolved
@ClausHolbechArista ClausHolbechArista merged commit 6b27b2c into aristanetworks:devel Dec 17, 2024
43 checks passed
@ClausHolbechArista ClausHolbechArista deleted the refactor/plugins/improve-schema-models branch December 17, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
one approval This PR has one approval and is only missing one more. rn: Refactor(plugins) role: eos_designs issue related to eos_designs role state: Documentation role Updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants