-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add AsyncTyper class to the Python SDK #2453
Conversation
Disable too-many-statements in pylint because it's already covered by ruff
@@ -12,6 +11,7 @@ | |||
from rich.table import Table | |||
|
|||
from infrahub_sdk import Error, GraphQLError | |||
from infrahub_sdk.async_typer import AsyncTyper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why we don't want to have this under infrahub_sdk.ctl instead? Is the intention that the server components would also load the same class from this location?. To me it feels cleaner to have it under .ctl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to have it available for other cli tools not just infrahubctl
@@ -205,6 +208,7 @@ ignore = [ | |||
"PLR0912", # Too many branches | |||
"PLR0913", # Too many arguments in function definition | |||
"PLR0917", # Too many positional arguments | |||
"PLR2004", # Magic value used in comparison |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually have any violations that motivates adding this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I'm seeing a lot of error when I run invoke sdk.format sdk.lint
locally
After more investigation it looks like the command ruff check --fix python_sdk --config python_sdk/pyproject.toml
doesn't respect the configuration file :/
Fixes #2354
This PR adds a new AsyncTyper class to the SDK and converts some
infrahubctl
commands that are using async to using this new version of Typer.As @ogenstad suggested in slack, technically most
infrahubctl
commands could be converted to using the Sync client. The main reason I would like to have it in there is because theinfrahub
cli requires Async for most commands so it will be very useful there and to simplify the creation of other cli tools as well.