Skip to content

Conversation

FragmentedPacket
Copy link
Contributor

Fixes #138

Added group_names_raw to provide the group name slug rather than prepending with the group_by option.

e.g. Before

group_by:
  - device_roles

device_roles_core-switch

e.g. After

group_by:
  - device_roles

core-switch

This also replaces any dash in the slug to an underscore since Ansible will be enforcing that moving from 2.10+. I'm open to keeping it as just returning the slug so please discuss what you guys think is the best approach.

@FragmentedPacket
Copy link
Contributor Author

@smolz @DouglasHeriot @TawR1024 @Yannis100

Can I get feedback from y'all on this change?

@Yannis100
Copy link

Thanks for asking!
I would rather prefer an option to include prefix or not, and by default have it like before
The option to have the prefix shrinked/shortened to a specific length could be great too, it's the way I implemented it in my "enhanced" version of the inventory plugin

@DouglasHeriot
Copy link
Contributor

My concern with this is that without the prefixes, all the slugs share the same namespace. However I can't think of any concrete examples where this will create an issue (ie. a manufacturer with the same name as a site)

As long as there's an option to turn it on/off I think this is ok.

I think in our usage of this we might leave the prefixes on as the verbosity helps when reading configurations to understand what's happening. And when running ansible-inventory to see what's there it is a lot clearer when sorted by all the site groups together, all the manufacturer groups together, etc.

The group names "manufacturers_apple" and "platforms_apple_ios" are a lot clearer than "apple" and "apple_ios" - I can much more easily explain to colleages where to find this thing in netbox.

Another thought - there's no reason that both sets of groups couldn't co-exist. Could help people transition, but would also increase confusion.

@ThomasADavis
Copy link
Contributor

We use the group names to build up a genders file - and with the roles prefixed, that file gets ugly quite fast.

as for the '-' to '_', ansible is being stupid in my mind.. I'd like a flag to keep them not converted (we already do the ansible.cfg flag to turn off the warning)

@FragmentedPacket
Copy link
Contributor Author

Alright. It seems like keeping the way it does it now should stay long term (forever) and adding the new option would also stick around and not replace the current method. I will also remove any data manipulation when it comes to changing from a dash to underscore as Ansible provides the config option to ignore the warnings.

@Yannis100 Do you mind providing me an example of the shortening?

smolz
smolz previously approved these changes Apr 3, 2020
Copy link

@smolz smolz left a comment

Choose a reason for hiding this comment

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

Think this gets everyone what they want. As long as I have the option to disable the prepending of the group name I am happy.

@FragmentedPacket FragmentedPacket merged commit fdf247e into netbox-community:devel Apr 6, 2020
@FragmentedPacket FragmentedPacket deleted the inv-group-names branch April 6, 2020 19:33
@Yannis100
Copy link

Yannis100 commented Apr 7, 2020

You can find it in this branch of my fork
https://github.com/Yannis100/ansible/blob/region-group-netbox/lib/ansible/plugins/inventory/netbox.py
ansible/ansible#60124
Haven't work on ansible for a while so not sure if it's updated with upstream

@DouglasHeriot
Copy link
Contributor

@Yannis100 I have to say thanks for your fork - I've been using your inventory for a little while and am now working on moving some features (the ones I need at least) into this project. I'll be doing regions next...

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.

Remove or provide option to NOT append the "group_by" group name to inventory groups
5 participants