Skip to content

Commit

Permalink
ci: Use ruff for linting and format (#199)
Browse files Browse the repository at this point in the history
- Code cleanup with ruff
- Fix typos

---------

Co-authored-by: haidaraM <[email protected]>
  • Loading branch information
haidaraM and haidaraM authored Sep 8, 2024
1 parent f9e1534 commit 76dbdac
Show file tree
Hide file tree
Showing 29 changed files with 1,059 additions and 1,011 deletions.
22 changes: 16 additions & 6 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
name: Lint
name: Format and lint

on:
push:
branches:
- main
paths:
- '**.py'
- ruff.toml
- tests/requirements_tests.txt
pull_request:
branches:
- main
paths:
- '**.py'
- ruff.toml
- tests/requirements_tests.txt

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand All @@ -20,16 +24,22 @@ permissions:
contents: write

jobs:
black:
ruff:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- uses: psf/black@stable
- uses: actions/setup-python@v5
name: Setup Python 3.11
with:
version: "~= 24.0" # https://black.readthedocs.io/en/stable/integrations/github_actions.html
options: ""
python-version: 3.11
cache: pip
cache-dependency-path: tests/requirements_tests.txt

- run: pip install -q -r tests/requirements_tests.txt

- run: make lint

- uses: stefanzweifel/git-auto-commit-action@v5
with:
commit_message: Autoformat code using black
commit_message: Auto lint and format using ruff
5 changes: 3 additions & 2 deletions .github/workflows/testing.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ jobs:
name: Setup Python ${{ matrix.python-version }}
with:
python-version: ${{ matrix.python-version }}
cache: pip

- name: Install prereqs
run: |
pip install -q ansible-core=='${{ matrix.ansible-core-version }}' virtualenv setuptools wheel coveralls
pip install -qr tests/requirements_tests.txt
pip install -q -r requirements.txt -r tests/requirements_tests.txt
pip freeze
sudo apt-get install -yq graphviz
ansible-galaxy install -r tests/fixtures/requirements.yml
Expand Down Expand Up @@ -89,7 +90,7 @@ jobs:
COVERALLS_PARALLEL: true

coveralls:
runs-on: ubuntu-20.04
runs-on: ubuntu-latest
name: Finish coverage
needs: pytest
container: python:3-slim # just need a simple python container to finish the coverage
Expand Down
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ deploy_test: clean build
test_install: build
@./tests/test_install.sh $(VIRTUALENV_DIR) $(ANSIBLE_CORE_VERSION)

fmt:
black .
lint:
ruff format
ruff check --fix

test: fmt
test:
# Due to some side effects with Ansible, we have to run the tests in a certain order
cd tests && pytest test_cli.py test_utils.py test_parser.py test_graph_model.py test_graphviz_postprocessor.py test_graphviz_renderer.py test_mermaid_renderer.py test_json_renderer.py

Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ More information [here](https://docs.ansible.com/ansible/latest/reference_append
Contributions are welcome. Feel free to contribute by creating an issue or submitting a PR :smiley:
### Dev environment
### Local development
To setup a new development environment :
To setup a new local development environment :
- Install graphviz (see above)
- (cd tests && pip install -r requirements_tests.txt)
- pip install -r requirements.txt -r tests/requirements_tests.txt
Run the tests and open the generated files in your system’s default viewer application:
Expand Down
5 changes: 5 additions & 0 deletions ansibleplaybookgrapher/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
"""A command line tool to create a graph representing your Ansible playbook tasks and roles.
While you can use this package into another project, it is not primarily designed for that (yet).
"""

__version__ = "2.3.0"
__prog__ = "ansible-playbook-grapher"
80 changes: 49 additions & 31 deletions ansibleplaybookgrapher/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.
import json
import ntpath
import os
import sys
from argparse import Namespace
from collections.abc import Callable
from pathlib import Path

from ansible.cli import CLI
from ansible.cli.arguments import option_helpers
Expand All @@ -29,24 +31,26 @@
from ansibleplaybookgrapher.renderer.graphviz import GraphvizRenderer
from ansibleplaybookgrapher.renderer.json import JSONRenderer
from ansibleplaybookgrapher.renderer.mermaid import (
MermaidFlowChartRenderer,
DEFAULT_DIRECTIVE as MERMAID_DEFAULT_DIRECTIVE,
)
from ansibleplaybookgrapher.renderer.mermaid import (
DEFAULT_ORIENTATION as MERMAID_DEFAULT_ORIENTATION,
)
from ansibleplaybookgrapher.renderer.mermaid import (
MermaidFlowChartRenderer,
)

# The display is a singleton. This instruction will NOT return a new instance.
# We explicitly set the verbosity after the init.
display = Display()


class PlaybookGrapherCLI(CLI):
"""
The dedicated playbook grapher CLI
"""
"""The dedicated playbook grapher CLI."""

name = __prog__

def __init__(self, args, callback=None):
def __init__(self, args: list[str], callback: Callable | None = None) -> None:
super().__init__(args=args, callback=callback)
# We keep the old options as instance attribute for backward compatibility for the grapher CLI.
# From Ansible 2.8, they remove this instance attribute 'options' and use a global context instead.
Expand All @@ -55,7 +59,7 @@ def __init__(self, args, callback=None):
self.options = None

def run(self):
# FIXME: run should not return anything.
# TODO(haidaraM): run should not return anything.
super().run()

display.verbosity = self.options.verbosity
Expand Down Expand Up @@ -112,13 +116,13 @@ def run(self):

case _:
# Likely a bug if we are here
msg = f"Unknown renderer '{self.options.renderer}'. This is likely a bug that should be reported."
raise AnsibleOptionsError(
f"Unknown renderer '{self.options.renderer}'. This is likely a bug that should be reported."
msg,
)

def _add_my_options(self):
"""
Add some of my options to the parser
def _add_my_options(self) -> None:
"""Add some of my options to the parser.
:return:
"""
self.parser.prog = __prog__
Expand Down Expand Up @@ -152,7 +156,7 @@ def _add_my_options(self):
"--view",
action="store_true",
default=False,
help="Automatically open the resulting SVG file with your systems default viewer application for the file type",
help="Automatically open the resulting SVG file with your system's default viewer application for the file type",
)

self.parser.add_argument(
Expand All @@ -168,11 +172,11 @@ def _add_my_options(self):
dest="open_protocol_handler",
choices=list(OPEN_PROTOCOL_HANDLERS.keys()),
default="default",
help="""The protocol to use to open the nodes when double-clicking on them in your SVG
viewer (only for graphviz). Your SVG viewer must support double-click and Javascript.
The supported values are 'default', 'vscode' and 'custom'.
For 'default', the URL will be the path to the file or folders. When using a browser,
it will open or download them.
help="""The protocol to use to open the nodes when double-clicking on them in your SVG
viewer (only for graphviz). Your SVG viewer must support double-click and Javascript.
The supported values are 'default', 'vscode' and 'custom'.
For 'default', the URL will be the path to the file or folders. When using a browser,
it will open or download them.
For 'vscode', the folders and files will be open with VSCode.
For 'custom', you need to set a custom format with --open-protocol-custom-formats.
""",
Expand All @@ -186,12 +190,12 @@ def _add_my_options(self):
--open-protocol-handler is set to custom.
You should provide a JSON formatted string like: {"file": "", "folder": ""}.
Example: If you want to open folders (roles) inside the browser and files (tasks) in
vscode, set it to:
vscode, set it to:
'{"file": "vscode://file/{path}:{line}:{column}", "folder": "{path}"}'.
path: the absolute path to the file containing the the plays/tasks/roles.
line/column: the position of the plays/tasks/roles in the file.
You can optionally add the attribute "remove_from_path" to remove some parts of the
path if you want relative paths.
line/column: the position of the plays/tasks/roles in the file.
You can optionally add the attribute "remove_from_path" to remove some parts of the
path if you want relative paths.
""",
)

Expand Down Expand Up @@ -255,7 +259,19 @@ def _add_my_options(self):
option_helpers.add_vault_options(self.parser)
option_helpers.add_runtask_options(self.parser)

def init_parser(self, usage="", desc=None, epilog=None):
def init_parser(
self,
usage: str | None = "",
desc: str | None = None,
epilog: str | None = None,
) -> None:
"""Create an options parser for the grapher.
:param usage:
:param desc:
:param epilog:
:return:
"""
super().init_parser(
usage=f"{__prog__} [options] playbook.yml",
desc="Make graphs from your Ansible Playbooks.",
Expand All @@ -264,7 +280,7 @@ def init_parser(self, usage="", desc=None, epilog=None):

self._add_my_options()

def post_process_args(self, options):
def post_process_args(self, options: Namespace) -> Namespace:
options = super().post_process_args(options)

# init the options
Expand All @@ -273,7 +289,7 @@ def post_process_args(self, options):
if self.options.output_filename is None:
basenames = map(ntpath.basename, self.options.playbook_filenames)
basenames_without_ext = "-".join(
[os.path.splitext(basename)[0] for basename in basenames]
[Path(basename).stem for basename in basenames],
)
self.options.output_filename = basenames_without_ext

Expand All @@ -282,30 +298,32 @@ def post_process_args(self, options):

return options

def validate_open_protocol_custom_formats(self):
"""
Validate the provided open protocol format
def validate_open_protocol_custom_formats(self) -> None:
"""Validate the provided open protocol format.
:return:
"""
error_msg = 'Make sure to provide valid formats. Example: {"file": "vscode://file/{path}:{line}:{column}", "folder": "{path}"}'
format_str = self.options.open_protocol_custom_formats
if not format_str:
raise AnsibleOptionsError(
msg = (
"When the protocol handler is to set to custom, you must provide the formats to "
"use with --open-protocol-custom-formats."
)
raise AnsibleOptionsError(
msg,
)
try:
format_dict = json.loads(format_str)
except Exception as e:
display.error(
f"{type(e).__name__} when reading the provided formats '{format_str}': {e}"
f"{type(e).__name__} when reading the provided formats '{format_str}': {e}",
)
display.error(error_msg)
sys.exit(1)

if "file" not in format_dict or "folder" not in format_dict:
display.error(
f"The field 'file' or 'folder' is missing from the provided format '{format_str}'"
f"The field 'file' or 'folder' is missing from the provided format '{format_str}'",
)
display.error(error_msg)
sys.exit(1)
Expand All @@ -314,7 +332,7 @@ def validate_open_protocol_custom_formats(self):
self.options.open_protocol_custom_formats = format_dict


def main(args=None):
def main(args: list[str] | None = None) -> None:
args = args or sys.argv
cli = PlaybookGrapherCLI(args)

Expand Down
Loading

0 comments on commit 76dbdac

Please sign in to comment.