Type stub generator for operations.

[StefanBRas]

As mentioned in this issue a way to solve the typing issues on functions with the @operation decorator is to generate type stubs.

This PR introduces that. I have not tested it anymore than it looks to be working and pyright isn't complaining:

When using operations, they will now not complain about the global args but still complain if any unknown arg is added:

The generator functions are in the file scripts/generate_operation_type_stubs.py and running python scripts/generate_operation_type_stubs.py will generate a stub for each *.py file in pyright/operations/

This could (should) be extended to also have the correct annotations on each global arg, but I wanted to hear if it is of interrest at all before moving forward.

It introduces a dev dependency on the package "redbaron". It could probably be done with any other AST package or ast from standard lib. I only ended up on RedBaron because it seemed to be the most user friendly and documented one.

[Fizzadar]

This is fantastic @StefanBRas! Definitely want to get this included in pyinfra. Definitely agree it'd be good to add types for global args.

Fun fact: pyinfra used to use RedBaron to compile operation files to work around operation ordering with conditional statements (since fixed with stack call ordering): bd6c48f#diff-fb2026268ae4b6a8ad26e8f37bf78bb7ff9a0af57ef13b2d571e3200cb02e5d5 - so absolutely OK with the extra dev dependency :)

[StefanBRas]

Great! Right now I just get the argument names from OPERATION_KWARGS in arguments.py, which doesn't have the types. Should I infer them from the default values or are they actually typed somewhere?

Apart adding those types and linting, is there anything else that would need to be added/changed?

It could be added as a part of the build process, but I'm not sure how you would want to integrate it.

Also, maybe the generator should be included as a util function, since operation authors would probably also like to generate stubs for their own operations.

[Fizzadar]

Great! Right now I just get the argument names from OPERATION_KWARGS in arguments.py, which doesn't have the types. Should I infer them from the default values or are they actually typed somewhere?

They're not currently typed anywhere, could add them to the definitions or just infer from the defaults for now, both seem reasonable to me. Perhaps look for a "type" field in the spec falling back to the default? ie explicit type would be needed here:

"_env": {
    "description": "Dictionary of environment variables to set.",
    "handler": generate_env,
    "type": dict[str, str],
},

I'm also totally happy to do this part, up to you :)

Apart adding those types and linting, is there anything else that would need to be added/changed?

Only thing would be making all the stub functions use one line per argument (just spotted some are on one super long line). Other than that this is good to go I think.

Long run I'm keen to add type annotations within the operations themselves, and I see this nicely handles that and includes them in the stubs which is ideal.

It could be added as a part of the build process, but I'm not sure how you would want to integrate it.

Yeah I've been trying to figure out this part, I think first up will be adding a CI lint test to the GitHub workflow that checks the stubs are up to date which could just generate them and check if any files have changed. That should ensure the stubs are up to date similar to linting.

Also, maybe the generator should be included as a util function, since operation authors would probably also like to generate stubs for their own operations.

Agreed, I've got a local folder in the repo pyinfra_util/ that may be a suitable place for this (don't think it belongs within pyinfra/ or pyinfra_cli/); perhaps a pyinfra_util/stubgen.py that could be used with python -m pyinfra_util.stubgen?

[StefanBRas]

They're not currently typed anywhere, could add them to the definitions or just infer from the defaults for now, both seem reasonable to me. Perhaps look for a "type" field in the spec falling back to the default? ie explicit type would be needed here:

"_env": {
    "description": "Dictionary of environment variables to set.",
    "handler": generate_env,
    "type": dict[str, str],
},

I'm also totally happy to do this part, up to you :)

Ah, with "infer from the defaults" I actually ment manually infering them with my mind and then adding it. I will give it a try and then just add a comment if some of them are not clear to me.

Only thing would be making all the stub functions use one line per argument (just spotted some are on one super long line). Other than that this is good to go I think.

Yeah, not sure why that happens actually. It seems almost random. I'll look into it.

Agreed, I've got a local folder in the repo pyinfra_util/ that may be a suitable place for this (don't think it belongs within pyinfra/ or pyinfra_cli/); perhaps a pyinfra_util/stubgen.py that could be used with python -m pyinfra_util.stubgen?

Sounds good! I will let it take an optional argument so that you can point to other operations, right now it's hardcoded to point to the operations.py from pyinfra.

[codecov]

Codecov Report

Merging #817 (0112214) into 2.x (a9a42cc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              2.x     #817   +/-   ##
=======================================
  Coverage   91.68%   91.69%           
=======================================
  Files         122      122           
  Lines        7519     7526    +7     
=======================================
+ Hits         6894     6901    +7     
  Misses        625      625           

Impacted Files	Coverage Δ
pyinfra/api/arguments.py	100.00% <100.00%> (ø)
pyinfra/operations/apk.py	100.00% <100.00%> (ø)
pyinfra/operations/util/files.py	100.00% <0.00%> (ø)
pyinfra/operations/server.py	96.98% <0.00%> (+0.02%)	⬆️
pyinfra/operations/files.py	98.78% <0.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9a42cc...0112214. Read the comment docs.

[StefanBRas]

@Fizzadar I implemented the changes now. For some reason RedBaron would format it as one line when the operation decorator was used with parenthsis and multiple lines if not, I have no idea why. I just use black programmaticlly to fix the formatting, that seemed easier than investigating.

I make some TODO comments in pyinfra/api/arguments.py at the places where I'm not sure about the types. You should probably check all the types, but especially those.

[Fizzadar]

Thanks for doing this so quick @StefanBRas! Have added suggestions and run through all the types!

[StefanBRas]

No problem! I added the fixes from the code review - unfortunantly this gives a circular import on Host as you can see in the pipeline. Normaly this could be solved with a if TYPE_CHECKING import guard, but since they are used at "runtime" (stub-generation-time) that's not possible here.

I'm not sure if it's best to loosen the type or refactor but in any case I think that's easier for you to do.

I think you should be able to directly edit the branch right?

[Fizzadar]

Awesome, thanks @StefanBRas! I pushed up a workaround for now, not ideal but means the type can stay in place and avoids the circular import.