Skip to content

Commit

Permalink
Replace black & isort with ruff for formatting (#930)
Browse files Browse the repository at this point in the history
  • Loading branch information
ogenstad authored Jun 27, 2024
1 parent ff2d26e commit 7cac01b
Show file tree
Hide file tree
Showing 25 changed files with 77 additions and 292 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ jobs:

- name: Run ruff
run: make ruff
- name: Run black
run: make black
- name: Run mypy
run: make mypy
- name: Run sphinx
Expand Down
10 changes: 5 additions & 5 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The guidelines to pin dependencies are:
a. if semver is supported we pin to major release
b. if semver is not supported we pin to specific version
2. For development:
a. black is pinned to a specific version
a. ruff is pinned to a specific version
b. everything is set to *

Then, to update them:
Expand Down Expand Up @@ -109,17 +109,17 @@ You can then stop them with:
Coding style
------------

Nornir uses `Black <https://github.com/ambv/black>`_, the uncompromising Python code formatter. Black makes it easy for you to format your code as you can do so automatically after installing it.
Nornir uses `Ruff <https://docs.astral.sh/ruff/>`_. Ruff makes it easy for you to format your code as you can do so automatically after installing it.

.. code-block:: bash
poetry run black .
poetry run ruff format --check .
The Black GitHub repo has information about how you can integrate Black in your editor.
The Ruff GitHub repo has information about how you can integrate Ruff in your editor.

Tests
-------------
As part of the automatic CI on every pull request, besides coding style checks with ``black``, we also do linting with ``ruff``, static type checking with ``mypy``, unit tests with ``pytest``, docs generation with ``sphinx`` and ``nbsphinx`` (for Jupyter notebooks) and verification of outputs in Jupyter notebook tutorials with pytest plugin ``nbval``.
As part of the automatic CI on every pull request, besides coding style checks and linting with ``ruff``, static type checking with ``mypy``, unit tests with ``pytest``, docs generation with ``sphinx`` and ``nbsphinx`` (for Jupyter notebooks) and verification of outputs in Jupyter notebook tutorials with pytest plugin ``nbval``.

After modifying any code in the core, at first, we recommend running unit tests locally before running the whole test suite (which takes longer time):

Expand Down
7 changes: 1 addition & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ docker:
pytest:
poetry run pytest --cov=nornir --cov-report=term-missing -vs ${ARGS}

.PHONY: black
black:
poetry run black --check ${NORNIR_DIRS}
poetry run isort --profile black --check ${NORNIR_DIRS}

.PHONY: sphinx
sphinx:
# TODO REPLACE with: sphinx-build -n -E -q -N -b dummy -d docs/_build/doctrees docs asd
Expand All @@ -40,7 +35,7 @@ ruff:
poetry run ruff check .

.PHONY: tests
tests: ruff black mypy nbval pytest sphinx
tests: ruff mypy nbval pytest sphinx

.PHONY: docker-tests
docker-tests: docker
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
![Build Status](https://github.com/nornir-automation/nornir/workflows/test%20nornir/badge.svg) [![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/ambv/black) [![Coverage Status](https://coveralls.io/repos/github/nornir-automation/nornir/badge.svg?branch=develop)](https://coveralls.io/github/nornir-automation/nornir?branch=develop)
![Build Status](https://github.com/nornir-automation/nornir/workflows/test%20nornir/badge.svg) [![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![Coverage Status](https://coveralls.io/repos/github/nornir-automation/nornir/badge.svg?branch=develop)](https://coveralls.io/github/nornir-automation/nornir?branch=develop)


Nornir
Expand Down
4 changes: 1 addition & 3 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,7 @@
# Grouping the document tree into LaTeX files. List of tuples
# (source start file, target name, title,
# author, documentclass [howto, manual, or own class]).
latex_documents = [
(master_doc, "nornir.tex", "nornir Documentation", "David Barroso", "manual")
]
latex_documents = [(master_doc, "nornir.tex", "nornir Documentation", "David Barroso", "manual")]


# -- Options for manual page output ---------------------------------------
Expand Down
8 changes: 2 additions & 6 deletions nornir/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ def with_processors(self, processors: List[Processor]) -> "Nornir":
Given a list of Processor objects return a copy of the nornir object with the processors
assigned to the copy. The original object is left unmodified.
"""
return Nornir(
**{**self._clone_parameters(), **{"processors": Processors(processors)}}
)
return Nornir(**{**self._clone_parameters(), **{"processors": Processors(processors)}})

def with_runner(self, runner: RunnerPlugin) -> "Nornir":
"""
Expand Down Expand Up @@ -145,9 +143,7 @@ def run(
result = self.runner.run(run_task, run_on)

raise_on_error = (
raise_on_error
if raise_on_error is not None
else self.config.core.raise_on_error
raise_on_error if raise_on_error is not None else self.config.core.raise_on_error
) # noqa
if raise_on_error:
result.raise_on_error()
Expand Down
22 changes: 6 additions & 16 deletions nornir/core/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ class SSHConfig(object):
__slots__ = ("config_file",)

class Parameters:
config_file = Parameter(
default=DEFAULT_SSH_CONFIG, envvar="NORNIR_SSH_CONFIG_FILE"
)
config_file = Parameter(default=DEFAULT_SSH_CONFIG, envvar="NORNIR_SSH_CONFIG_FILE")

def __init__(self, config_file: Optional[str] = None) -> None:
self.config_file = self.Parameters.config_file.resolve(config_file)
Expand All @@ -73,13 +71,9 @@ class InventoryConfig(object):
__slots__ = "plugin", "options", "transform_function", "transform_function_options"

class Parameters:
plugin = Parameter(
typ=str, default="SimpleInventory", envvar="NORNIR_INVENTORY_PLUGIN"
)
plugin = Parameter(typ=str, default="SimpleInventory", envvar="NORNIR_INVENTORY_PLUGIN")
options = Parameter(default={}, envvar="NORNIR_INVENTORY_OPTIONS")
transform_function = Parameter(
typ=str, envvar="NORNIR_INVENTORY_TRANSFORM_FUNCTION"
)
transform_function = Parameter(typ=str, envvar="NORNIR_INVENTORY_TRANSFORM_FUNCTION")
transform_function_options = Parameter(
default={}, envvar="NORNIR_INVENTORY_TRANSFORM_FUNCTION_OPTIONS"
)
Expand All @@ -93,13 +87,9 @@ def __init__(
) -> None:
self.plugin = self.Parameters.plugin.resolve(plugin)
self.options = self.Parameters.options.resolve(options) or {}
self.transform_function = self.Parameters.transform_function.resolve(
transform_function
)
self.transform_function_options = (
self.Parameters.transform_function_options.resolve(
transform_function_options
)
self.transform_function = self.Parameters.transform_function.resolve(transform_function)
self.transform_function_options = self.Parameters.transform_function_options.resolve(
transform_function_options
)

def dict(self) -> Dict[str, Any]:
Expand Down
12 changes: 3 additions & 9 deletions nornir/core/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ def __init__(self, **kwargs: Any) -> None:
self.filters = kwargs

def __call__(self, host: Host) -> bool:
return all(
F._verify_rules(host, k.split("__"), v) for k, v in self.filters.items()
)
return all(F._verify_rules(host, k.split("__"), v) for k, v in self.filters.items())

def __and__(self, other: "F") -> AND:
return AND(self, other)
Expand Down Expand Up @@ -106,16 +104,12 @@ def _verify_rules(data: Any, rule: List[str], value: Any) -> bool:
return bool(data.get(rule[0]) == value)

else:
raise Exception(
"I don't know how I got here:\n{}\n{}\n{}".format(data, rule, value)
)
raise Exception("I don't know how I got here:\n{}\n{}\n{}".format(data, rule, value))


class NOT_F(F):
def __call__(self, host: Host) -> bool:
return not any(
F._verify_rules(host, k.split("__"), v) for k, v in self.filters.items()
)
return not any(F._verify_rules(host, k.split("__"), v) for k, v in self.filters.items())

def __invert__(self) -> F:
return F(**self.filters)
Expand Down
9 changes: 2 additions & 7 deletions nornir/core/helpers/jinja_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,10 @@


def render_from_file(
path: str,
template: str,
jinja_filters: Optional[Dict[str, Any]] = None,
**kwargs: Any
path: str, template: str, jinja_filters: Optional[Dict[str, Any]] = None, **kwargs: Any
) -> str:
jinja_filters = jinja_filters or {}
env = Environment(
loader=FileSystemLoader(path), undefined=StrictUndefined, trim_blocks=True
)
env = Environment(loader=FileSystemLoader(path), undefined=StrictUndefined, trim_blocks=True)
env.filters.update(jinja_filters)
t = env.get_template(template)
return t.render(**kwargs)
Expand Down
32 changes: 9 additions & 23 deletions nornir/core/inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,7 @@ def dict(self) -> Dict[str, Any]:
return {
"groups": [g.name for g in self.groups],
"data": self.data,
"connection_options": {
k: v.dict() for k, v in self.connection_options.items()
},
"connection_options": {k: v.dict() for k, v in self.connection_options.items()},
**super().dict(),
}

Expand Down Expand Up @@ -230,9 +228,7 @@ def schema(self) -> Dict[str, Any]:
def dict(self) -> Dict[str, Any]:
return {
"data": self.data,
"connection_options": {
k: v.dict() for k, v in self.connection_options.items()
},
"connection_options": {k: v.dict() for k, v in self.connection_options.items()},
**super().dict(),
}

Expand Down Expand Up @@ -298,9 +294,7 @@ def schema(cls) -> Dict[str, Any]:
def dict(self) -> Dict[str, Any]:
return {
"name": self.name,
"connection_options": {
k: v.dict() for k, v in self.connection_options.items()
},
"connection_options": {k: v.dict() for k, v in self.connection_options.items()},
**super().dict(),
}

Expand Down Expand Up @@ -405,9 +399,7 @@ def get(self, item: str, default: Any = None) -> Any:
except KeyError:
return default

def get_connection_parameters(
self, connection: Optional[str] = None
) -> ConnectionOptions:
def get_connection_parameters(self, connection: Optional[str] = None) -> ConnectionOptions:
if not connection:
d = ConnectionOptions(
hostname=self.hostname,
Expand Down Expand Up @@ -439,9 +431,7 @@ def get_connection_parameters(
)
return d

def _get_connection_options_recursively(
self, connection: str
) -> Optional[ConnectionOptions]:
def _get_connection_options_recursively(self, connection: str) -> Optional[ConnectionOptions]:
p = self.connection_options.get(connection)
if p is None:
p = ConnectionOptions(None, None, None, None, None, None)
Expand Down Expand Up @@ -579,13 +569,11 @@ class Groups(Dict[str, Group]):


class TransformFunction(Protocol):
def __call__(self, host: Host, **kwargs: Any) -> None:
...
def __call__(self, host: Host, **kwargs: Any) -> None: ...


class FilterObj(Protocol):
def __call__(self, host: Host, **kwargs: Any) -> bool:
...
def __call__(self, host: Host, **kwargs: Any) -> bool: ...


class Inventory(object):
Expand All @@ -607,13 +595,11 @@ def filter(
self,
filter_obj: Optional[FilterObj] = None,
filter_func: Optional[FilterObj] = None,
**kwargs: Any
**kwargs: Any,
) -> "Inventory":
filter_func = filter_obj or filter_func
if filter_func:
filtered = Hosts(
{n: h for n, h in self.hosts.items() if filter_func(h, **kwargs)}
)
filtered = Hosts({n: h for n, h in self.hosts.items() if filter_func(h, **kwargs)})
else:
filtered = Hosts(
{
Expand Down
4 changes: 1 addition & 3 deletions nornir/core/plugins/runners.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,4 @@ def run(self, task: Task, hosts: List[Host]) -> AggregatedResult:
raise NotImplementedError("needs to be implemented by the plugin")


RunnersPluginRegister: PluginRegister[Type[RunnerPlugin]] = PluginRegister(
RUNNERS_PLUGIN_PATH
)
RunnersPluginRegister: PluginRegister[Type[RunnerPlugin]] = PluginRegister(RUNNERS_PLUGIN_PATH)
16 changes: 4 additions & 12 deletions nornir/core/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ def task_instance_started(self, task: Task, host: Host) -> None:
"""
raise NotImplementedError("needs to be implemented by the processor")

def task_instance_completed(
self, task: Task, host: Host, result: MultiResult
) -> None:
def task_instance_completed(self, task: Task, host: Host, result: MultiResult) -> None:
"""
This method is called when a host completes its instance of a task
"""
Expand All @@ -44,9 +42,7 @@ def subtask_instance_started(self, task: Task, host: Host) -> None:
"""
raise NotImplementedError("needs to be implemented by the processor")

def subtask_instance_completed(
self, task: Task, host: Host, result: MultiResult
) -> None:
def subtask_instance_completed(self, task: Task, host: Host, result: MultiResult) -> None:
"""
This method is called when a host completes executing a subtask
"""
Expand Down Expand Up @@ -76,18 +72,14 @@ def task_instance_started(self, task: Task, host: Host) -> None:
for p in self:
p.task_instance_started(task, host)

def task_instance_completed(
self, task: Task, host: Host, result: MultiResult
) -> None:
def task_instance_completed(self, task: Task, host: Host, result: MultiResult) -> None:
for p in self:
p.task_instance_completed(task, host, result)

def subtask_instance_started(self, task: Task, host: Host) -> None:
for p in self:
p.subtask_instance_started(task, host)

def subtask_instance_completed(
self, task: Task, host: Host, result: MultiResult
) -> None:
def subtask_instance_completed(self, task: Task, host: Host, result: MultiResult) -> None:
for p in self:
p.subtask_instance_completed(task, host, result)
4 changes: 1 addition & 3 deletions nornir/core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ class GlobalState(object):

__slots__ = "dry_run", "failed_hosts"

def __init__(
self, dry_run: bool = False, failed_hosts: Optional[Set[str]] = None
) -> None:
def __init__(self, dry_run: bool = False, failed_hosts: Optional[Set[str]] = None) -> None:
self.dry_run = dry_run
self.failed_hosts = failed_hosts or set()

Expand Down
12 changes: 5 additions & 7 deletions nornir/core/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def __init__(
name: Optional[str] = None,
severity_level: int = DEFAULT_SEVERITY_LEVEL,
parent_task: Optional["Task"] = None,
**kwargs: str
**kwargs: str,
):
self.task = task
self.nornir = nornir
Expand All @@ -68,7 +68,7 @@ def copy(self) -> "Task":
self.name,
self.severity_level,
self.parent_task,
**self.params
**self.params,
)

def __repr__(self) -> str:
Expand Down Expand Up @@ -163,7 +163,7 @@ def grouped_tasks(task):
global_dry_run=self.global_dry_run,
processors=self.processors,
parent_task=self,
**kwargs
**kwargs,
)
r = run_task.start(self.host)
self.results.append(r[0] if len(r) == 1 else cast("Result", r))
Expand Down Expand Up @@ -213,7 +213,7 @@ def __init__(
failed: bool = False,
exception: Optional[BaseException] = None,
severity_level: int = DEFAULT_SEVERITY_LEVEL,
**kwargs: Any
**kwargs: Any,
):
self.result = result
self.host = host
Expand Down Expand Up @@ -286,9 +286,7 @@ def __init__(self, name: str, **kwargs: MultiResult):
super().__init__(**kwargs)

def __repr__(self) -> str:
return "{} ({}): {}".format(
self.__class__.__name__, self.name, super().__repr__()
)
return "{} ({}): {}".format(self.__class__.__name__, self.name, super().__repr__())

@property
def failed(self) -> bool:
Expand Down
4 changes: 1 addition & 3 deletions nornir/plugins/inventory/simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ def _get_inventory_element(
password=data.get("password"),
platform=data.get("platform"),
data=data.get("data"),
groups=data.get(
"groups"
), # this is a hack, we will convert it later to the correct type
groups=data.get("groups"), # this is a hack, we will convert it later to the correct type
defaults=defaults,
connection_options=_get_connection_options(data.get("connection_options", {})),
)
Expand Down
Loading

0 comments on commit 7cac01b

Please sign in to comment.