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(plugins): Add python classes built from schemas #4603

Draft
wants to merge 57 commits into
base: devel
Choose a base branch
from

Conversation

ClausHolbechArista
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista commented Oct 16, 2024

Change Summary

Add python classes built from schemas

Component(s) name

arista.avd.eos_designs
arista.avd.eos_cli_config_gen

Proposed changes

  • New tooling to generate Python classes from AVD schemas.
  • Loader to load data including coercion into the new classes.
  • Modify various eos_designs plugins and classes to load inputs into an EosDesigns class instance.
  • Modify connected endpoints logic in shared_utils, facts and structured_config to take advantage of the EosDesigns class - as a POC.
  • Minor fixes of schemas.

How to test

No changes to molecule.

Checklist

User Checklist

  • N/A

Repository Checklist

  • My code has been rebased from devel before I start
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation and documentation have been updated accordingly.
  • I have updated molecule CI testing accordingly. (check the box if not applicable)

@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role type: code quality CI and development toolset state: Documentation role Updated role: eos_designs issue related to eos_designs role labels Oct 16, 2024
Copy link

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-4603
# Activate the virtual environment
source test-avd-pr-4603/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/ClausHolbechArista/avd.git@schemas-as-classes-no-validation-nested#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/,schemas-as-classes-no-validation-nested --force
# Optional: Install AVD examples
cd test-avd-pr-4603
ansible-playbook arista.avd.install_examples

@github-actions github-actions bot added the state: conflict PR with conflict label Oct 17, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from 2ca3b9b to bfeca85 Compare October 18, 2024 06:47
@github-actions github-actions bot removed the state: conflict PR with conflict label Oct 18, 2024
Copy link

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

@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from b103dd9 to 032572f Compare October 18, 2024 17:37
@github-actions github-actions bot added the state: conflict PR with conflict label Oct 21, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

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

@github-actions github-actions bot removed the state: conflict PR with conflict label Oct 21, 2024
@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from 6de1bee to e101cc8 Compare October 22, 2024 07:10
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the state: conflict PR with conflict label Oct 23, 2024
@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from 15b7c65 to c0188b3 Compare October 29, 2024 15:29
@github-actions github-actions bot removed the state: conflict PR with conflict label Oct 29, 2024
Copy link

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

@github-actions github-actions bot added state: conflict PR with conflict state: CI Updated CI scenario have been updated in the PR and removed state: conflict PR with conflict labels Oct 29, 2024
Copy link

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

@github-actions github-actions bot added the state: conflict PR with conflict label Oct 30, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

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.

very partial review so far ona couple of classes will carry on

pylintrc Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved


class AvdBase:
_is_avd_class: bool = True
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed? Could we not just use isinstance(foo, AvdBase)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You will see it later in other functions where we wish to avoid circular imports.

python-avd/pyavd/_schema/models/avd_base.py Outdated Show resolved Hide resolved
Comment on lines +57 to +61
if value is not None and not isinstance(value, field_type):
msg = f"Invalid type '{type(value)}. Expected '{field_type}'. Value '{value}"
raise TypeError(msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

so it means we revalidate the type here in addtition to in the schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should never happen as long as the schema tooling ran first. But yes, it is a safeguard in case of validation running with warnings and we cannot coerce.

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_model.py Outdated Show resolved Hide resolved
python-avd/pyavd/_schema/models/avd_model.py Outdated Show resolved Hide resolved
@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from fa548bb to 58413d5 Compare November 5, 2024 09:59
@github-actions github-actions bot removed the state: conflict PR with conflict label Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

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

python-avd/pyproject.toml Outdated Show resolved Hide resolved
python-avd/schema_tools/generate_classes/src_generators.py Outdated Show resolved Hide resolved

dict_fields_src = [f'"type": {field_type}']
if field_type == "list":
items_type = self.type_hints[0].list_item_type
Copy link
Contributor

Choose a reason for hiding this comment

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

so here it assumes that the first type_hints is always a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes since field_type comes from the same place it will always be the case.

"""

name: str
type_hint: FieldTypeHintSrc
Copy link
Contributor

Choose a reason for hiding this comment

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

so no Union here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you mean we only accept a single type hint for the class var, and yes that is all we need.

python-avd/schema_tools/generate_classes/src_generators.py Outdated Show resolved Hide resolved
python-avd/schema_tools/generate_classes/src_generators.py Outdated Show resolved Hide resolved
if field.description:
description = (
"\n".join(wrap(field.description, width=100, replace_whitespace=False))
.replace("Example:\n", "Example: # fmt: skip\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

why the fmt: skip ? is it for ruff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we have any of these things in our descriptions, ruff picks it up as a docstring heading, so it messes up everything. Took quite some fiddling to get this working :)

python-avd/schema_tools/generate_classes/src_generators.py Outdated Show resolved Hide resolved
@ClausHolbechArista ClausHolbechArista force-pushed the schemas-as-classes-no-validation-nested branch from dc8300a to 3016e5a Compare November 6, 2024 14:37
Copy link

sonarcloud bot commented Nov 7, 2024

@github-actions github-actions bot added the state: conflict PR with conflict label Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_cli_config_gen issue related to eos_cli_config_gen role role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR state: conflict PR with conflict state: Documentation role Updated type: code quality CI and development toolset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants