Skip to content

plugin signature based on options#69947

Draft
bcoca wants to merge 11 commits intoansible:develfrom
bcoca:plugin_sigs
Draft

plugin signature based on options#69947
bcoca wants to merge 11 commits intoansible:develfrom
bcoca:plugin_sigs

Conversation

@bcoca
Copy link
Member

@bcoca bcoca commented Jun 8, 2020

Allow same plugin class with same options to be 'the same'

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jun 8, 2020
@bcoca bcoca mentioned this pull request Jun 8, 2020
Closed
@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Jun 8, 2020
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jun 8, 2020
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jun 9, 2020
@samdoran samdoran changed the title hash plugins based on options [WIP] hash plugins based on options Jun 9, 2020
@ansibot ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Jun 9, 2020
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 19, 2020
@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 5, 2020
@ansibot ansibot removed the pre_azp This PR was last tested before migration to Azure Pipelines. label Apr 29, 2021
@bcoca bcoca changed the title [WIP] hash plugins based on options plugin signature based on options Apr 29, 2021
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Apr 29, 2021
@ansible ansible deleted a comment from ansibot Apr 29, 2021
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. labels Apr 29, 2021
@ansibot
Copy link
Contributor

ansibot commented May 11, 2022

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

lib/ansible/plugins/connection/__init__.py:106:5: E303: too many blank lines (2)

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

lib/ansible/plugins/__init__.py:66:59: undefined-variable: Undefined variable 'variables'

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 11, 2022
s-hertel
s-hertel reviewed May 11, 2022
View reviewed changes
lib/ansible/plugins/__init__.py

def _gen_signature(self):
# create immutable
s_options = pickle.dumps(self.get_options(hostvars=variables))
Copy link
Contributor

@s-hertel s-hertel May 11, 2022

Choose a reason for hiding this comment

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

If I copy an inventory config file and use both, should the plugin have the same hash, or be unique per source? The latter happens now with pickle.dumps.

Copy link
Member Author

@bcoca bcoca May 11, 2022

Choose a reason for hiding this comment

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

would need the same as connections it's own _gen_signature to include 'source path'

Copy link
Contributor

@s-hertel s-hertel May 11, 2022

Choose a reason for hiding this comment

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

Yeah, that would make sense. It seems like it includes source path now (implicitly) since the hashes are different, but I'm not sure exactly what's happening.

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label May 16, 2022
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 24, 2022
@bcoca bcoca mentioned this pull request Aug 3, 2022
Closed
1 task
@bcoca bcoca marked this pull request as draft August 24, 2022 20:15
@ansibot ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Aug 24, 2022
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Sep 1, 2022
@ansibot ansibot removed the has_issue label Jul 12, 2023
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_pr This PR has not been pushed to for more than one year. labels Jun 4, 2025
@ansibot
Copy link
Contributor

ansibot commented Jun 4, 2025

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

lib/ansible/plugins/connection/__init__.py:121:5: E303: too many blank lines (2)

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/plugins/__init__.py:119:59: undefined-variable: Undefined variable 'variables'
lib/ansible/plugins/__init__.py:123:15: unnecessary-dunder-call: Unnecessarily calls dunder method __hash__. Use hash built-in function.
lib/ansible/plugins/callback/__init__.py:209:59: undefined-variable: Undefined variable 'get_plugin_class'

click here for bot help

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s-hertel s-hertel s-hertel left review comments

Assignees

No one assigned

Labels

affects_2.10 This issue/PR affects Ansible v2.10 feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team. WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bcoca @mattclay @ansibot @s-hertel @samdoran