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

WIP DNM Handle network platform action plugin collection searching #61489

Closed

Conversation

@cidrblock
Copy link
Contributor

commented Aug 28, 2019

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

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

lib/ansible/executor/task_executor.py:1129:1: E303 too many blank lines (3)

click here for bot help

Bradley A. Thornton added 3 commits Aug 28, 2019
Bradley A. Thornton
Bradley A. Thornton
Bradley A. Thornton
if self._shared_loader_obj.action_loader.has_plugin(self._task.action, collection_list=collections):
collection_from_task, _, module_name = self._task.action.rpartition(".")
module_prefix = module_name.split("_")[0]
if collection_from_task:

This comment has been minimized.

Copy link
@Qalthos

Qalthos Aug 28, 2019

Contributor

We should be able to merge the two cases with something like this?

if collections is None and collection_from_task:
    collections = [collection_from_task]

Now collections is either the non-None contents of collections, or the task's collection, or None if neither exist, which means we can drop the collections check from the all()s and just use collections regardless

This comment has been minimized.

Copy link
@cidrblock

cidrblock Aug 28, 2019

Author Contributor

I would be worried about changing the collection list in this case for a non-network module....
(because we would have changed the collection when we didn't have a network module match)

This comment has been minimized.

Copy link
@cidrblock

cidrblock Aug 28, 2019

Author Contributor

that might be ok though, I don't think the first case below (non network related) would pick up a handler in the same collection as the module unless the user explicitly listed that collection...

This comment has been minimized.

Copy link
@cidrblock

cidrblock Aug 28, 2019

Author Contributor

and IMHO, we should be using the handler in the collection local to the module by default if one exists...

This comment has been minimized.

Copy link
@Qalthos

Qalthos Aug 28, 2019

Contributor

Ah, I hadn't noticed collections isn't added here.

And yeah, we should probably be using something like collection_list=[collection_from_task] + collections to look up the action plugin (resolving appropriately when one or both don't exist). The two paths are just so similar to each other I'd really rather avoid keeping them separate unless there's a good reason to

This comment has been minimized.

Copy link
@cidrblock

cidrblock Aug 28, 2019

Author Contributor

Hmmm. That raises an interesting question. If a user provides a collection list, should we search that before or after the collection local to the module. I would think we search the collection local to the module after explicitly listed collections but before defaulting to a core found plugin. I can't see how to do that without 2 steps. Thoughts?

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

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

lib/ansible/executor/task_executor.py:1024:30: blacklisted-name: Black listed name "_"

click here for bot help

Bradley A. Thornton
In case handler was not found in collection list
- Look in the collection local to the module

@cidrblock cidrblock closed this Aug 29, 2019

@sivel sivel removed the needs_triage label Aug 29, 2019

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