From 2905c7d1aed3c46f9fb007c38af7a4c7e7c54d52 Mon Sep 17 00:00:00 2001 From: Karel van Klink <karel.vanklink@geant.org> Date: Wed, 15 Nov 2023 17:49:06 +0100 Subject: [PATCH] resolve last linting errors --- gso/api/__init__.py | 1 + gso/api/v1/__init__.py | 1 + gso/api/v1/imports.py | 1 + gso/api/v1/subscriptions.py | 1 + gso/cli/import_sites.py | 1 + gso/cli/netbox.py | 1 + gso/products/__init__.py | 1 + gso/products/product_blocks/iptrunk.py | 4 ++-- gso/products/product_blocks/router.py | 1 + gso/products/product_blocks/site.py | 1 + gso/services/netbox_client.py | 1 + gso/services/provisioning_proxy.py | 1 + gso/utils/helpers.py | 2 +- gso/workflows/__init__.py | 1 + gso/workflows/iptrunk/create_iptrunk.py | 2 +- gso/workflows/iptrunk/migrate_iptrunk.py | 8 ++------ .../iptrunk/modify_trunk_interface.py | 4 ++-- pyproject.toml | 11 +++-------- test/conftest.py | 2 +- test/schedules/test_scheduling.py | 19 +++++++++++++------ test/utils/test_helpers.py | 8 ++++---- test/workflows/iptrunk/test_create_iptrunk.py | 6 +++--- .../workflows/router/test_terminate_router.py | 2 +- test/workflows/site/test_create_site.py | 7 +++++-- test/workflows/site/test_modify_site.py | 2 +- test/workflows/site/test_terminate_site.py | 2 +- tox.ini | 8 +++----- utils/netboxcli.py | 7 ++++--- 28 files changed, 59 insertions(+), 47 deletions(-) diff --git a/gso/api/__init__.py b/gso/api/__init__.py index edcd2c75..d6167385 100644 --- a/gso/api/__init__.py +++ b/gso/api/__init__.py @@ -1,4 +1,5 @@ """Initialisation class for the :term:`GSO` :term:`API`.""" + from fastapi import APIRouter from gso.api.v1 import router as router_v1 diff --git a/gso/api/v1/__init__.py b/gso/api/v1/__init__.py index 05d4e8c7..c14de2e3 100644 --- a/gso/api/v1/__init__.py +++ b/gso/api/v1/__init__.py @@ -1,4 +1,5 @@ """Version 1 of the :term:`GSO` :term:`API`.""" + from fastapi import APIRouter from gso.api.v1.imports import router as imports_router diff --git a/gso/api/v1/imports.py b/gso/api/v1/imports.py index fcc4a89a..e87f4ad8 100644 --- a/gso/api/v1/imports.py +++ b/gso/api/v1/imports.py @@ -1,4 +1,5 @@ """:term:`GSO` :term:`API` endpoints that import different types of existing services.""" + import ipaddress from typing import Any from uuid import UUID diff --git a/gso/api/v1/subscriptions.py b/gso/api/v1/subscriptions.py index fee0cbb6..4e6e3d2a 100644 --- a/gso/api/v1/subscriptions.py +++ b/gso/api/v1/subscriptions.py @@ -1,4 +1,5 @@ """:term:`API` endpoint for fetching different types of subscriptions.""" + from typing import Any from fastapi import Depends, status diff --git a/gso/cli/import_sites.py b/gso/cli/import_sites.py index c897ae7a..21182766 100644 --- a/gso/cli/import_sites.py +++ b/gso/cli/import_sites.py @@ -1,4 +1,5 @@ """:term:`CLI` command for importing sites.""" + import typer app: typer.Typer = typer.Typer() diff --git a/gso/cli/netbox.py b/gso/cli/netbox.py index 958684cb..b4a8b1b6 100644 --- a/gso/cli/netbox.py +++ b/gso/cli/netbox.py @@ -1,4 +1,5 @@ """A :term:`CLI` for interacting with Netbox.""" + import typer from pynetbox import RequestError diff --git a/gso/products/__init__.py b/gso/products/__init__.py index 9fa0448d..74f8fa15 100644 --- a/gso/products/__init__.py +++ b/gso/products/__init__.py @@ -4,6 +4,7 @@ Whenever a new product type is added, this should be reflected in the :py:class:`gso.products.ProductType` enumerator. """ + from orchestrator.domain import SUBSCRIPTION_MODEL_REGISTRY from pydantic_forms.types import strEnum diff --git a/gso/products/product_blocks/iptrunk.py b/gso/products/product_blocks/iptrunk.py index 41b1ee64..3bf5266b 100644 --- a/gso/products/product_blocks/iptrunk.py +++ b/gso/products/product_blocks/iptrunk.py @@ -36,7 +36,7 @@ class IptrunkType(strEnum): T_co = TypeVar("T_co", covariant=True) -class LAGMemberList(UniqueConstrainedList[T_co]): +class LAGMemberList(UniqueConstrainedList[T_co]): # type: ignore[type-var] """A list of :term:`LAG` member interfaces.""" @@ -66,7 +66,7 @@ class IptrunkInterfaceBlock(IptrunkInterfaceBlockProvisioning, lifecycle=[Subscr interface_description: str -class IptrunkSides(UniqueConstrainedList[T_co]): +class IptrunkSides(UniqueConstrainedList[T_co]): # type: ignore[type-var] """A list of IP trunk interfaces that make up one side of a link.""" min_items = 2 diff --git a/gso/products/product_blocks/router.py b/gso/products/product_blocks/router.py index d902a8fe..8f0896a5 100644 --- a/gso/products/product_blocks/router.py +++ b/gso/products/product_blocks/router.py @@ -1,4 +1,5 @@ """Product block for :class:`Router` products.""" + import ipaddress from orchestrator.domain.base import ProductBlockModel diff --git a/gso/products/product_blocks/site.py b/gso/products/product_blocks/site.py index 89c63768..1950d297 100644 --- a/gso/products/product_blocks/site.py +++ b/gso/products/product_blocks/site.py @@ -1,4 +1,5 @@ """The product block that describes a site subscription.""" + import re from orchestrator.domain.base import ProductBlockModel diff --git a/gso/services/netbox_client.py b/gso/services/netbox_client.py index 5781393a..e8e2bdf4 100644 --- a/gso/services/netbox_client.py +++ b/gso/services/netbox_client.py @@ -1,4 +1,5 @@ """Contain all methods to communicate with the NetBox API endpoint. Data Center Infrastructure Main (DCIM).""" + from uuid import UUID import pydantic diff --git a/gso/services/provisioning_proxy.py b/gso/services/provisioning_proxy.py index d979b687..51e942ab 100644 --- a/gso/services/provisioning_proxy.py +++ b/gso/services/provisioning_proxy.py @@ -2,6 +2,7 @@ :term:`LSO` is responsible for executing Ansible playbooks, that deploy subscriptions. """ + import json import logging from functools import partial diff --git a/gso/utils/helpers.py b/gso/utils/helpers.py index 37ff1d87..d0df5745 100644 --- a/gso/utils/helpers.py +++ b/gso/utils/helpers.py @@ -213,7 +213,7 @@ def validate_site_name(site_name: str) -> str: if not pattern.match(site_name): msg = ( "Enter a valid site name. It must consist of three uppercase letters (A-Z) followed by an optional single " - "digit (0-9)." + f"digit (0-9). Received: {site_name}" ) raise ValueError(msg) return site_name diff --git a/gso/workflows/__init__.py b/gso/workflows/__init__.py index 451afaa9..845f5c2b 100644 --- a/gso/workflows/__init__.py +++ b/gso/workflows/__init__.py @@ -1,4 +1,5 @@ """Initialisation class that imports all workflows into :term:`GSO`.""" + from orchestrator.workflows import LazyWorkflowInstance LazyWorkflowInstance("gso.workflows.iptrunk.create_iptrunk", "create_iptrunk") diff --git a/gso/workflows/iptrunk/create_iptrunk.py b/gso/workflows/iptrunk/create_iptrunk.py index c5bf342e..a0a5af27 100644 --- a/gso/workflows/iptrunk/create_iptrunk.py +++ b/gso/workflows/iptrunk/create_iptrunk.py @@ -174,7 +174,7 @@ def create_subscription(product: UUIDstr, customer: UUIDstr) -> State: @step("Get information from IPAM") def get_info_from_ipam(subscription: IptrunkProvisioning) -> State: - """Allocate :term:`IP` resources in :term:`IPAM`.""" + """Allocate IP resources in :term:`IPAM`.""" subscription.iptrunk.iptrunk_ipv4_network = infoblox.allocate_v4_network( "TRUNK", subscription.iptrunk.iptrunk_description, diff --git a/gso/workflows/iptrunk/migrate_iptrunk.py b/gso/workflows/iptrunk/migrate_iptrunk.py index 8f122fa2..383a8189 100644 --- a/gso/workflows/iptrunk/migrate_iptrunk.py +++ b/gso/workflows/iptrunk/migrate_iptrunk.py @@ -301,9 +301,7 @@ def confirm_continue_move_fiber() -> FormGenerator: class Config: title = "Please confirm before continuing" - info_label: Label = ( - "New trunk interface has been deployed, wait for the physical connection to be moved." # type: ignore[assignment] - ) + info_label: Label = "New trunk interface has been deployed, wait for the physical connection to be moved." # type: ignore[assignment] yield ProvisioningResultPage @@ -354,9 +352,7 @@ def confirm_continue_restore_isis() -> FormGenerator: class Config: title = "Please confirm before continuing" - info_label: Label = ( - "ISIS config has been deployed, confirm if you want to restore the old metric." # type: ignore[assignment] - ) + info_label: Label = "ISIS config has been deployed, confirm if you want to restore the old metric." # type: ignore[assignment] yield ProvisioningResultPage diff --git a/gso/workflows/iptrunk/modify_trunk_interface.py b/gso/workflows/iptrunk/modify_trunk_interface.py index d2326a92..3c3e5662 100644 --- a/gso/workflows/iptrunk/modify_trunk_interface.py +++ b/gso/workflows/iptrunk/modify_trunk_interface.py @@ -39,8 +39,8 @@ def initialize_ae_members(subscription: Iptrunk, initial_user_input: dict, side_ iptrunk_speed = initial_user_input["iptrunk_speed"] class NokiaLAGMember(LAGMember): - interface_name: ( - available_interfaces_choices_including_current_members( # type: ignore[valid-type] + interface_name: ( # type: ignore[valid-type] + available_interfaces_choices_including_current_members( router.owner_subscription_id, iptrunk_speed, subscription.iptrunk.iptrunk_sides[side_index].iptrunk_side_ae_members, diff --git a/pyproject.toml b/pyproject.toml index 7a618cf8..90528c79 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,14 +26,8 @@ disable_error_code = "annotation-unchecked" enable_error_code = "ignore-without-code" [tool.ruff] -exclude = [ - ".git", - ".*_cache", - ".tox", - "*.egg-info", - "__pycache__", +extend-exclude = [ "htmlcov", - "venv", "gso/migrations", "docs", ] @@ -42,7 +36,8 @@ ignore = [ "D213", "N805", "PLR0913", - "PLR0904" + "PLR0904", + "PLW1514" ] line-length = 120 select = [ diff --git a/test/conftest.py b/test/conftest.py index a9bf269b..262ed33f 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -186,7 +186,7 @@ def data_config_filename(configuration_data) -> str: yield f.name del os.environ["OSS_PARAMS_FILENAME"] - os.remove(f.name) + Path(f.name).unlink() @pytest.fixture(scope="session") diff --git a/test/schedules/test_scheduling.py b/test/schedules/test_scheduling.py index 496b3673..5ed2ad01 100644 --- a/test/schedules/test_scheduling.py +++ b/test/schedules/test_scheduling.py @@ -8,7 +8,7 @@ from gso.schedules.scheduling import scheduler @pytest.fixture(scope="module") def validate_subscriptions(): - from gso.schedules.validate_subscriptions import validate_subscriptions as vs + from gso.schedules.validate_subscriptions import validate_subscriptions as vs # noqa: PLC0415 return vs @@ -32,7 +32,7 @@ def mock_logger(): yield mock -@pytest.fixture +@pytest.fixture() def mock_celery(): with patch("gso.schedules.scheduling.current_app") as mock_app: yield mock_app @@ -85,7 +85,9 @@ def test_no_subscriptions(mock_get_insync_subscriptions, mock_logger, validate_s def test_subscriptions_without_system_target_workflow( - mock_get_insync_subscriptions, mock_logger, validate_subscriptions + mock_get_insync_subscriptions, + mock_logger, + validate_subscriptions, ): mock_get_insync_subscriptions.return_value = [MagicMock(product=MagicMock(workflows=[]))] validate_subscriptions() @@ -93,7 +95,9 @@ def test_subscriptions_without_system_target_workflow( def test_subscription_status_not_usable( - mock_get_insync_subscriptions, mock_get_execution_context, validate_subscriptions + mock_get_insync_subscriptions, + mock_get_execution_context, + validate_subscriptions, ): subscription_mock = MagicMock() subscription_mock.product.workflows = [MagicMock(target=Target.SYSTEM, name="workflow_name")] @@ -107,7 +111,9 @@ def test_subscription_status_not_usable( def test_valid_subscriptions_for_validation( - mock_get_insync_subscriptions, mock_get_execution_context, validate_subscriptions + mock_get_insync_subscriptions, + mock_get_execution_context, + validate_subscriptions, ): subscription_mock = MagicMock() mocked_workflow = MagicMock(target=Target.SYSTEM, name="workflow_name") @@ -117,5 +123,6 @@ def test_valid_subscriptions_for_validation( validate_subscriptions() validate_func = mock_get_execution_context()["validate"] validate_func.assert_called_once_with( - mocked_workflow.name, json=[{"subscription_id": str(subscription_mock.subscription_id)}] + mocked_workflow.name, + json=[{"subscription_id": str(subscription_mock.subscription_id)}], ) diff --git a/test/utils/test_helpers.py b/test/utils/test_helpers.py index f8375a48..0e93c2aa 100644 --- a/test/utils/test_helpers.py +++ b/test/utils/test_helpers.py @@ -7,14 +7,14 @@ from gso.products.product_blocks.router import RouterVendor from gso.utils.helpers import available_interfaces_choices_including_current_members -@pytest.fixture +@pytest.fixture() def mock_router(): """Fixture to mock the Router class.""" with patch("gso.utils.helpers.Router") as mock: yield mock -@pytest.fixture +@pytest.fixture() def mock_netbox_client(): """Fixture to mock the NetboxClient class.""" with patch("gso.utils.helpers.NetboxClient") as mock: @@ -40,7 +40,7 @@ def test_nokia_router_with_interfaces_returns_choice(mock_router, mock_netbox_cl [ {"name": "interface1", "module": {"display": "module1"}, "description": "desc1"}, {"name": "interface2", "module": {"display": "module2"}, "description": "desc2"}, - ] + ], ) mock_netbox_client().get_interface_by_name_and_device.return_value = { "name": "interface3", @@ -53,7 +53,7 @@ def test_nokia_router_with_interfaces_returns_choice(mock_router, mock_netbox_cl interface_description="desc3", owner_subscription_id=faker.uuid4(), subscription_instance_id=faker.uuid4(), - ) + ), ] result = available_interfaces_choices_including_current_members(faker.uuid4(), "10G", interfaces) diff --git a/test/workflows/iptrunk/test_create_iptrunk.py b/test/workflows/iptrunk/test_create_iptrunk.py index 336344d9..bc6cf64d 100644 --- a/test/workflows/iptrunk/test_create_iptrunk.py +++ b/test/workflows/iptrunk/test_create_iptrunk.py @@ -90,7 +90,6 @@ def input_form_wizard_data(router_subscription_factory, faker): @pytest.mark.workflow() -@pytest.mark.usefixtures(_netbox_client_mock) @patch("gso.workflows.iptrunk.create_iptrunk.provisioning_proxy.check_ip_trunk") @patch("gso.workflows.iptrunk.create_iptrunk.provisioning_proxy.provision_ip_trunk") @patch("gso.workflows.iptrunk.create_iptrunk.infoblox.allocate_v6_network") @@ -103,6 +102,7 @@ def test_successful_iptrunk_creation_with_standard_lso_result( responses, input_form_wizard_data, faker, + _netbox_client_mock, # noqa: PT019 data_config_filename: PathLike, test_client, ): @@ -129,7 +129,6 @@ def test_successful_iptrunk_creation_with_standard_lso_result( @pytest.mark.workflow() -@pytest.mark.usefixtures(_netbox_client_mock) @patch("gso.workflows.iptrunk.create_iptrunk.provisioning_proxy.check_ip_trunk") @patch("gso.workflows.iptrunk.create_iptrunk.provisioning_proxy.provision_ip_trunk") @patch("gso.workflows.iptrunk.create_iptrunk.infoblox.allocate_v6_network") @@ -142,6 +141,7 @@ def test_iptrunk_creation_fails_when_lso_return_code_is_one( responses, input_form_wizard_data, faker, + _netbox_client_mock, # noqa: PT019 data_config_filename: PathLike, ): mock_allocate_v4_network.return_value = faker.ipv4_network() @@ -156,4 +156,4 @@ def test_iptrunk_creation_fails_when_lso_return_code_is_one( assert_pp_interaction_failure(result, process_stat, step_log) assert mock_check_ip_trunk.call_count == 0 - assert mock_provision_ip_trunk.call_count == 1 + assert mock_provision_ip_trunk.call_count == 2 diff --git a/test/workflows/router/test_terminate_router.py b/test/workflows/router/test_terminate_router.py index b7c31d98..0b8af2b2 100644 --- a/test/workflows/router/test_terminate_router.py +++ b/test/workflows/router/test_terminate_router.py @@ -36,7 +36,7 @@ def test_terminate_router_success( {"subscription_id": product_id}, router_termination_input_form_data, ] - result, process_stat, step_log = run_workflow("terminate_router", initial_router_data) + result, _, _ = run_workflow("terminate_router", initial_router_data) assert_complete(result) state = extract_state(result) diff --git a/test/workflows/site/test_create_site.py b/test/workflows/site/test_create_site.py index c49a6d87..0ddb06b0 100644 --- a/test/workflows/site/test_create_site.py +++ b/test/workflows/site/test_create_site.py @@ -28,7 +28,7 @@ def test_create_site(responses, faker): "customer": get_customer_by_name("GÉANT")["id"], }, ] - result, process, step_log = run_workflow("create_site", initial_site_data) + result, _, _ = run_workflow("create_site", initial_site_data) assert_complete(result) state = extract_state(result) @@ -51,7 +51,10 @@ def test_site_name_is_incorrect(responses, faker): The validation should throw an exception in case of a invalid string. """ invalid_site_name = "AMST10" - expected_exception_msg = f"Enter a valid site name similar looks like AMS, AMS1or LON9. Get: {invalid_site_name}" + expected_exception_msg = ( + r".*Enter a valid site name\. It must consist of three uppercase letters \(A-Z\) followed by an optional single" + rf" digit \(0\-9\)\. Received: {invalid_site_name} \(type=value_error\)" + ) product_id = get_product_id_by_name(ProductType.SITE) initial_site_data = [ {"product": product_id}, diff --git a/test/workflows/site/test_modify_site.py b/test/workflows/site/test_modify_site.py index 8465afcc..0db1a50f 100644 --- a/test/workflows/site/test_modify_site.py +++ b/test/workflows/site/test_modify_site.py @@ -16,7 +16,7 @@ def test_modify_site(responses, site_subscription_factory): "site_ts_address": "127.0.0.1", }, ] - result, process, step_log = run_workflow("modify_site", initial_site_data) + result, _, _ = run_workflow("modify_site", initial_site_data) assert_complete(result) state = extract_state(result) diff --git a/test/workflows/site/test_terminate_site.py b/test/workflows/site/test_terminate_site.py index 25347c9d..26cad9e2 100644 --- a/test/workflows/site/test_terminate_site.py +++ b/test/workflows/site/test_terminate_site.py @@ -8,7 +8,7 @@ from test.workflows import assert_complete, extract_state, run_workflow def test_terminate_site(responses, site_subscription_factory): subscription_id = site_subscription_factory() initial_site_data = [{"subscription_id": subscription_id}, {}] - result, process, step_log = run_workflow("terminate_site", initial_site_data) + result, _, _ = run_workflow("terminate_site", initial_site_data) assert_complete(result) state = extract_state(result) diff --git a/tox.ini b/tox.ini index 17883b4b..8ecdb7ee 100644 --- a/tox.ini +++ b/tox.ini @@ -10,15 +10,13 @@ setenv = OAUTH2_ACTIVE = False deps = coverage - mypy - ruff types-requests celery-stubs -r requirements.txt commands = - ruff --preview . - ruff format --respect-gitignore --preview . + ruff --respect-gitignore --preview . + ruff format --respect-gitignore --preview --check . mypy . coverage erase coverage run --source gso --omit="gso/migrations/*" -m pytest {posargs} @@ -27,4 +25,4 @@ commands = sh -c "if [ $SKIP_ALL_TESTS -eq 1 ]; then echo 'Skipping coverage report'; else coverage report --fail-under 80; fi" allowlist_externals = - sh \ No newline at end of file + sh diff --git a/utils/netboxcli.py b/utils/netboxcli.py index a4d42500..2cf2c1d7 100644 --- a/utils/netboxcli.py +++ b/utils/netboxcli.py @@ -1,5 +1,6 @@ """Command line tool to communicate with the NetBox API.""" -from typing import Any + +from typing import Any, List # noqa: UP035, List import needed since we shadow the ``list`` builtin as a command import click import pandas as pd @@ -7,9 +8,9 @@ import pandas as pd from gso.services.netbox_client import NetboxClient -def convert_to_table(data: list[dict[str, Any]], fields: list[str]) -> pd.DataFrame: +def convert_to_table(data: List[dict[str, Any]], fields: List[str]) -> pd.DataFrame: # noqa: UP006 """Convert raw data into a Pandas data table.""" - if not data: + if len(data) == 0: msg = "No data is available for your request" raise ValueError(msg) -- GitLab