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

Fix(eos_validate_state): ANTA Handle Pydantic + Python 3.9.7 bug gracefully #3601

Merged

Conversation

carl-baillargeon
Copy link
Contributor

Change Summary

  • Added try-except block to catch a known bug with Pydantic conint and Python 3.9.7, impacting ANTA. The bug manifests when loading Pydantic models using conint.

  • ANTA related tasks are now grouped in a specific file to avoid loading unnecessary ANTA modules when running validate state in NOT ANTA mode (use_anta=False).

  • Added a condition in AnsibleEOSDevice init to catch properly when ANTA is not installed or not imported.

Related Issue(s)

aristanetworks/anta#557

Component(s) name

arista.avd.eos_validate_state ANTA Mode

Proposed changes

How to test

Install Python 3.9.7 with ANTA 0.12.0 and run validate state in ANTA mode to see the bug:
image

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)

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.

Thanks for addressing this! a couple of comments :)

except TypeError as e:
# Known bug with Python 3.9.7 and Pydantic `conint`, impacting ANTA. Issue: https://github.com/arista-netdevops-community/anta/issues/557
if "Interval() takes no arguments" in str(e):
msg = "Encountered a known bug with Pydantic and Python 3.9.7. Please consider upgrading Python or using a different environment."
Copy link
Contributor

Choose a reason for hiding this comment

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

Users of AVD do not know anything about ANTA / Pydantic so the message may need to be more explicit

Something like

The ANTA testing framework leveraged in eos_validate_state is using a libary that is unable to load for some python 3.9.x versions (to the best of our knowledge).

Or something better said but we don't have a lot of info :). and probably invite them to a discussion on AVD / ANTA to report their python version where it failed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 359bec3

@@ -18,3 +18,26 @@
check_mode: false
when: use_anta | bool
Copy link
Contributor

Choose a reason for hiding this comment

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

so stupid question but do we still need the when, now that this can only be included when use_anta is true in the first place? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

and yeah I know this is from the one but I am thinking now that it can just be removed and same for the report task :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Fixed in 359bec3

@ankudinov ankudinov self-requested a review February 8, 2024 10:39
if "Interval() takes no arguments" in str(e):
msg = (
"The ANTA testing framework, used in the AVD eos_validate_state role, has encountered a compatibility issue with Python 3.9.x. "
"Please try a different Python version. For assistance or to report your Python version, please refer to the AVD/ANTA GitHub repositories:\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's add a known version where it works (3.9.13 or >=3.10)

Copy link
Contributor

Choose a reason for hiding this comment

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

Please if we are going to provide recommendation - something above 3.10. Although it definitely works with latest 3.9

Copy link
Contributor

Choose a reason for hiding this comment

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

it is not recommendation really more just a statement that if you are stuck on 3.9 for whatever reason on your system, you are not doomed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 44567d6

Copy link
Contributor

@ankudinov ankudinov left a comment

Choose a reason for hiding this comment

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

Tested. Looks good.

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.

LGTM

@ClausHolbechArista ClausHolbechArista merged commit 53ed794 into aristanetworks:devel Feb 9, 2024
38 checks passed
@carl-baillargeon carl-baillargeon deleted the fix/import_errors branch May 18, 2024 11:13
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.

4 participants