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

Adds duplicated code as function for inventory helper. #727

Closed

Conversation

jvanderaa
Copy link

Fixes #723

Details

  • Takes code that is repeated in multiple inventory files and adds it into the Nornir core inventory
  • Credit goes to Nornir NetBox and Nornir Ansible inventories that the content was sourced from
  • Adds tests to verify the information

https://github.com/wvandeun/nornir_netbox/blob/develop/nornir_netbox/plugins/inventory/netbox.py#L39-L48
https://github.com/carlmontanari/nornir_ansible/blob/master/nornir_ansible/plugins/inventory/ansible.py#L456-L472

@dbarrosop
Copy link
Contributor

I think if we want to go down this path and support this we should add a from_dict static method to all of the user facing objects (i.e. Host.from_dict(...), Group.from_dict(...)).

Basically that from_dict method should mean that a_host == Host.from_dict(a_host.dict()) with one exception; groups should be excluded and post-processed or you might end up in the situation where groups don't exist before trying to create the reference.

Thoughts?

@jvanderaa
Copy link
Author

That makes sense. It may take me a little bit longer to get back on that particular update. I also need to check on what is going on with the tests that are failing here now after locally testing.

@dbarrosop
Copy link
Contributor

Re failing tests, you may need to rebase as it's due to some breaking changes github actions did. Kirk fixed those already a few days ago though so hopefully you don't have to do anything about them.

@jvanderaa
Copy link
Author

I've still got this on my radar. I've just come down with a bit of a cold that has knocked me out of working much lately...

@dbarrosop
Copy link
Contributor

No worries, get well!

@jvanderaa
Copy link
Author

I'm getting this in front of me a bit more. I thought I had something but the tests are not working. So I am going to try to restart the process on another branch.

@ktbyers
Copy link
Collaborator

ktbyers commented Oct 3, 2023

@jvanderaa What is the status of this?

Is this dead now i.e. should be closed?

@jvanderaa
Copy link
Author

Yeah, probably good to close out. Sorry for leaving it for so long @ktbyers

@jvanderaa jvanderaa closed this Oct 3, 2023
@ktbyers
Copy link
Collaborator

ktbyers commented Oct 3, 2023

@jvanderaa No worries at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Inventory Defaults Loading
3 participants