Add Ruff as a code checker #222
Conversation
|
Available rules can be found here: https://beta.ruff.rs/docs/rules/ If you find one you think should be added, comment with your reasoning. |
JacobCallahan
force-pushed
the
0.4.0
branch
2 times, most recently
from
cb52d78 to
9532010
Compare
|
All tests passed locally, including functional for all providers. |
broker/helpers.py
Outdated
| filter_list = eval( | ||
| filter_list = eval( # noqa: S307 | ||
| raw_f.replace(f"@{filter_key}", filter_key), {filter_key: filter_list} | ||
| ) | ||
| filter_list = filter_list if isinstance(filter_list, list) else [filter_list] | ||
| filter_list = ( | ||
| filter_list if isinstance(filter_list, list) else [filter_list] | ||
| ) | ||
| elif f"@{filter_key}" in raw_f: | ||
| # perform an attribute filter on each host | ||
| filter_list = list( | ||
| filter( | ||
| lambda item: eval( | ||
| lambda item: eval( # noqa: S307 |
There was a problem hiding this comment.
S307 | suspicious-eval-usage | Use of possibly insecure function; consider using ast.literal_eval
Could you perhaps use ast.literal_eval?
There was a problem hiding this comment.
unfortunately, no. we need to pass in the filterable data structure and ast.literal_eval can't do that.
| if request := _frame.frame.f_locals.get("request"): | ||
| if request := _frame.frame.f_locals.get("request"): # noqa: F821 |
There was a problem hiding this comment.
Just me being curious, where is _frame being defined?
There was a problem hiding this comment.
This is largely due to the order in which the frames are available, so it gives us the ability to "look back" at the previous frame to get the request object from the actual fixture instead of the pytest-internal frame we'd be at otherwise.
ruff.toml
Outdated
| "D202", # No blank lines allowed after function docstring | ||
| "D203", # 1 blank line required before class docstring | ||
| "D213", # Multi-line docstring summary should start at the second line | ||
| "D406", # Section name should end with a newline |
There was a problem hiding this comment.
What is the reason for ignoring D406?
There was a problem hiding this comment.
See comment about D407 ;)
ruff.toml
Outdated
| # False positives https://github.com/astral-sh/ruff/issues/5386 | ||
| "PLC0208", # Use a sequence type instead of a `set` when iterating over values | ||
| # Ignored due to performance: https://github.com/charliermarsh/ruff/issues/2923 | ||
| "PLR0913", # Too many arguments to function call ({c_args} > {max_args}) |
There was a problem hiding this comment.
Isn't it better to define a sane number of max arguments?
https://beta.ruff.rs/docs/settings/#pylint-max-args
There was a problem hiding this comment.
Sane is up to personal preference and tool compatibility. Most of the times this was triggered in Broker was due to click command options.
ruff.toml
Outdated
| "D202", # No blank lines allowed after function docstring | ||
| "D203", # 1 blank line required before class docstring | ||
| "D213", # Multi-line docstring summary should start at the second line |
There was a problem hiding this comment.
Why can't broker be compliant with pydocstyle recommendations?
I would like to see at least D202 not being ignored.
There was a problem hiding this comment.
We can use D202, but I don't like the others.
Ruff is a newer code checker that is written in Rust and is very fast. Adding this now with a number of pretty sane checks to start with. I recommend having ruff integrated into your editor to pick out issues as you write code.
|
@ogajduse thanks for the thorough review! Updated all I saw to address and a little more. |
Switch all relevant config files to pyproject.toml Also added in pre-commit checks for black and ruff Bumped out python version 3.9 Added in dependabot config for actions, since others are largely unpinned
| try: | ||
| host_obj.teardown() | ||
| except Exception as err: | ||
| except Exception as err: # noqa: BLE001 | ||
| logger.debug(f"Tell Jake the exception was: {err}") |
There was a problem hiding this comment.
| logger.debug(f"Tell Jake the exception was: {err}") | |
| logger.exception(f"Tell Jake the exception was: {err}") |
There was a problem hiding this comment.
I'll keep as-is since the creation of the exception class below will also log as an error.
Ruff is a newer code checker that is written in Rust and is very fast. Adding this now with a number of pretty sane checks to start with. I recommend having Ruff integrated into your editor to pick out issues as you write code.
Most of the changes in this PR are adding/modifying docstrings.