-
Notifications
You must be signed in to change notification settings - Fork 29
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): NetBox as an inventory source #968
base: main
Are you sure you want to change the base?
Conversation
Also need to look at making the filter or search more dynamic. For example, maybe someone only wants the inventory from site=ny-dc-1. |
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! Thanks for taking this over! will need to add tests and documentation for this
@click.option("--nb-instance", "-nbi", help="Name of the NetBox instance", type=str, required=True) | ||
@click.option("--nb-token", "-nbt", help="NetBox token", type=str, required=True) | ||
@click.option("--nb-platform", "-nbp", help="NetBox device platform", type=str, default="Arista EOS", required=False) | ||
@click.option("--nb-site", "-nbs", help="NetBox site (case sensitive)", type=str, default=None, required=False) | ||
@click.option("--nb-verify", "-nbf", help="NetBox verify SSL", type=bool, default=False, required=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1/ Given that you are in the from-netbox command context I dont think you need to add nb
prefix to the options (except if it makes a lot of sense)
2/ the default for security should always be True
;)
3/ I think (and we may not be doing this everywhere) that you don't need required=False
as it is the default in click. Same for default=None
try: | ||
import pynetbox | ||
except ImportError as e: | ||
logging.error(e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make a better error message telling what to instal pip install anta[netbox]
?
except Exception as e: | ||
raise ValueError(e) from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to be more specific on which Exception you can get
Description
Initial implementation to get NetBox as an inventory source. This has a lot of areas where it could be improved (logging, syntax, naming) but I hope this initial PR can get the idea implemented.
Example using the public NetBox demo instance:
Still TODO:
Fixes #257
Checklist:
pre-commit run
)tox -e testenv
)