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

Incomplete type given in EDA plugin template #708

Open
3 tasks done
ssbarnea opened this issue Aug 28, 2024 · 1 comment
Open
3 tasks done

Incomplete type given in EDA plugin template #708

ssbarnea opened this issue Aug 28, 2024 · 1 comment

Comments

@ssbarnea
Copy link
Member

ssbarnea commented Aug 28, 2024

Please confirm the following

  • I agree to follow this project's code of conduct.
  • I have checked the current issues for duplicates.
  • I understand that ansible-rulebook is open source software provided for free and that I might not receive a timely response.

Bug Summary

The example given in https://ansible.readthedocs.io/projects/rulebook/en/latest/sources.html#plugin-template does have incomplete type signature (running mypy --strict would highlight the issues).

async def main(queue: asyncio.Queue, args: Dict[str, Any]):
  • First argument should be more like asyncio.Queue[Dict[str, Any])]
  • The args being just a generic Dict[str, Any] might not be the best type due to the use of Any - gives too much freedom and does not help plugin authors to easily write them. It would be desirable to be a more constrained type, if possible. Even defying a specific class for this purpose might help as plugins could import it for typing purposes.

Environment

Steps to reproduce

Actual results

Expected results

Complete type signature, maybe even including some examples of what kind of data would be received by the plugins.

Additional information

It should be noted that I found this while adding types to EDA collection as part of ansible/event-driven-ansible#258

If the data passed is generic JSON, it could prove useful to know that now we do have some JSON types compatible with mypy, we already defined them in https://github.com/ansible/ansible-compat/blob/main/src/ansible_compat/types.py#L13 as we needed them for several projects managed by devtools, feel free to copy them.

@Alex-Izquierdo
Copy link
Contributor

It is a good catch @ssbarnea
Actually, the payload passed through the queue as dict is going to be converted into a json afterwards, so we actually require the native types that are supported for a json string.

Then, the types that are defined in https://github.com/ansible/ansible-compat/blob/main/src/ansible_compat/types.py#L13 would be the right ones.

Thoughts? @ssbarnea @mkanoor

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

No branches or pull requests

3 participants
@ssbarnea @Alex-Izquierdo and others