Skip to content

playbook tasks restore module k/v for action#84479

Open
bcoca wants to merge 15 commits intoansible:develfrom
bcoca:restore_module_kv
Open

playbook tasks restore module k/v for action#84479
bcoca wants to merge 15 commits intoansible:develfrom
bcoca:restore_module_kv

Conversation

@bcoca
Copy link
Copy Markdown
Member

@bcoca bcoca commented Dec 19, 2024

ISSUE TYPE
  • Bugfix Pull Request

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 19, 2024
@ansible ansible deleted a comment from ansibot Dec 19, 2024
@ansible ansible deleted a comment from ansibot Dec 20, 2024
@bcoca bcoca changed the title playbook tasks restore action: module=actionname playbook tasks restore module k/v for action Dec 20, 2024
@ansible ansible deleted a comment from ansibot Dec 20, 2024
@ansible ansible deleted a comment from ansibot Dec 20, 2024
@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Dec 20, 2024
@ansible ansible deleted a comment from ansibot Dec 20, 2024
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Dec 20, 2024
@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 Jan 4, 2025
@ansible ansible deleted a comment from ansibot Jan 9, 2025
@bcoca bcoca force-pushed the restore_module_kv branch from fd8fba5 to e157716 Compare January 9, 2025 21:22
@ansible ansible deleted a comment from ansibot Jan 9, 2025
@ansibot ansibot removed 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. labels Jan 9, 2025
@bcoca bcoca force-pushed the restore_module_kv branch from e157716 to a7b371b Compare January 10, 2025 15:28
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 10, 2025
@ansible ansible deleted a comment from ansibot Jan 10, 2025
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 10, 2025
@bcoca bcoca marked this pull request as ready for review January 14, 2025 15:30
@bcoca bcoca added data_tagging and removed needs_triage Needs a first human triage before being processed. labels Jan 14, 2025
@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 Jan 19, 2025
@bcoca bcoca force-pushed the restore_module_kv branch from 7abd62e to 9a8738d Compare January 24, 2025 23:10
@ansibot ansibot removed 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 Jan 24, 2025
@bcoca bcoca force-pushed the restore_module_kv branch from 9a8738d to 14dab00 Compare February 3, 2025 19:08
@webknjaz
Copy link
Copy Markdown
Member

webknjaz commented Feb 4, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bcoca bcoca force-pushed the restore_module_kv branch from 14dab00 to fcde06c Compare April 14, 2025 18:05
@ansibot ansibot removed 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 Apr 14, 2025
@bcoca bcoca force-pushed the restore_module_kv branch from fcde06c to bc0fed2 Compare April 14, 2025 20:39
@webknjaz
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 15, 2025
return (action, final_args)

def _normalize_new_style_args(self, thing, action):
def _normalize_new_style_args(self, thing: dict[str, t.Any] | str | bytes | None, action: str) -> dict[str, t.Any] | None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's no bytes. Code below checks for string_types which is always (str,) on Python 3. So just keep str in there. Additionally, EncryptedString is missing.
Finally, it's possible to express some relations between args and the return value through type aliases:

Suggested change
def _normalize_new_style_args(self, thing: dict[str, t.Any] | str | bytes | None, action: str) -> dict[str, t.Any] | None:
PreParsedArgs: t.TypeAlias = dict[str, t.Any]
def _normalize_new_style_args(self, thing: PreParsedArgs | str | EncryptedString | None, action: str) -> PreParsedArgs | dict[str | t.Literal['_raw_params'], str | t.Any] | dict[t.Literal['_raw_params'], EncryptedString] | None:

(not tested, it's in-browser coding; move the PreParsedArgs definition to the top level)

return args

def _normalize_old_style_args(self, thing):
def _normalize_old_style_args(self, thing: dict[str, t.Any] | str | bytes | None) -> tuple[str, dict[str, t.Any]]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can also reuse that type alias here. And there's the same problem with bytes:

Suggested change
def _normalize_old_style_args(self, thing: dict[str, t.Any] | str | bytes | None) -> tuple[str, dict[str, t.Any]]:
def _normalize_old_style_args(self, thing: PreParsedArgs | str | None) -> tuple[str, PreParsedArgs]:

return (action, args)

def parse(self, skip_action_validation=False):
def parse(self, skip_action_validation: bool = False) -> tuple[str | None, dict[str, t.Any], str | Sentinel]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: this Sentinel thing is sneaky. We should really migrate to a typing-friendly solution that wouldn't accept subclasses...

Suggested change
def parse(self, skip_action_validation: bool = False) -> tuple[str | None, dict[str, t.Any], str | Sentinel]:
def parse(self, skip_action_validation: bool = False) -> tuple[str | None, PreParsedArgs, str | Sentinel]:

def _normalize_parameters(self, thing, action=None, additional_args=None):
def _normalize_parameters(
self,
thing: dict[str, t.Any] | str | bytes,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
thing: dict[str, t.Any] | str | bytes,
thing: PreParsedArgs | str,

self,
thing: dict[str, t.Any] | str | bytes,
action: str | None = None,
additional_args: str | dict[str, t.Any] | None = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
additional_args: str | dict[str, t.Any] | None = None
additional_args: str | EncryptedString | PreParsedArgs | None = None

thing: dict[str, t.Any] | str | bytes,
action: str | None = None,
additional_args: str | dict[str, t.Any] | None = None
) -> tuple[str, dict[str, t.Any]]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
) -> tuple[str, dict[str, t.Any]]:
) -> tuple[str, PreParsedArgs]:

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Apr 16, 2025
@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 Apr 29, 2025
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue/PR relates to a bug. ci_verified Changes made in this PR are causing tests to fail. data_tagging 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants