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

Removed additional fetch from config context because it is now available in the host from Netbox v2.6 onwards #59028

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@nikkytub
Copy link
Contributor

commented Jul 12, 2019

SUMMARY

Removed additional fetch from config context because it is now available in the host from Netbox v2.6 onwards.

Fixed #50704
Fixed #53393

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

config-context in hostvars in netbox dynamic inventory

ANSIBLE VERSION
ansible 2.8.2
  config file = /User/ansible/ansible/lib/ansible/plugins/inventory/ansible.cfg
  configured module search path = ['/User/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.7.3 (default, Mar 27 2019, 09:23:15) [Clang 10.0.1 (clang-1001.0.46.3)]
Removed additional fetch and config context as an option as it is now…
… available in the host from Netbox v2.6
@ansibot

This comment has been minimized.

@nikkytub

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

@FragmentedPacket @Anthony25 @pilou- @sieben Please review the changes

@FragmentedPacket

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

@nikkytub I would say we should at the very least include the option for Config Context still as it can degrade the performance of the plugin with a large inventory set and provides the users the option of disabling it.

We would need to change the URL's if it's False to have the following as a query parameter ?exclude=config_context.

@ansibot ansibot added python3 and removed needs_triage labels Jul 12, 2019

@nikkytub

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/plugins/inventory/netbox.py:384:53: E126 continuation line over-indented for hanging indent
lib/ansible/plugins/inventory/netbox.py:389:5: E303 too many blank lines (2)

click here for bot help

@nikkytub nikkytub force-pushed the nikkytub:config_context branch from 089fac9 to 0cb544c Jul 12, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/plugins/inventory/netbox.py:384:53: E126 continuation line over-indented for hanging indent

click here for bot help

@ansibot ansibot added the ci_verified label Jul 12, 2019

@nikkytub nikkytub force-pushed the nikkytub:config_context branch 2 times, most recently from 0b9dac4 to c5b26f7 Jul 12, 2019

@nikkytub nikkytub force-pushed the nikkytub:config_context branch from c5b26f7 to 7632ecd Jul 12, 2019

@nikkytub nikkytub changed the title Removed additional fetch and config context as an option because it is now available in the host from Netbox v2.6 onwards Removed additional fetch from config context because it is now available in the host from Netbox v2.6 onwards Jul 12, 2019

@FragmentedPacket
Copy link
Contributor

left a comment

Tested and works as intended

@FragmentedPacket

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2019

shipit

@ansibot ansibot added shipit and removed community_review labels Jul 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.