Skip to content

Commit

Permalink
Improve path handling, other small refactorings (Azure#39)
Browse files Browse the repository at this point in the history
# Main changes

## Make methods relying on self._tmp_dir private
- `self._tmp_dir` is only available in the context of calling `generate_nfd()`, so methods relying on `self._tmp_dir` should be private

## Use pathlib.Path rather than os file operations
- Provides clearer and stronger typing than passing `str`s around
- Adds some handy utility functions

## Variable renaming for clarity
- E.g. consistently use 'directory' / 'dir' (rather than mix with 'folder')
- Obvs somewhat subjective, but as someone new to most of this code, the changes made sense to me

## Add nfd_bicep_path as abstract property on NFDGenerator
- We rely on it when calling the concrete implementations
- Also use ABC rather than raise NotImplementedError

## Miscellaneous style updates to keep `azdev style aosm` happy
- isort
- black
  • Loading branch information
Cyclam authored Jul 4, 2023
1 parent 0f86392 commit 0497ec7
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 251 deletions.
20 changes: 11 additions & 9 deletions src/aosm/azext_aosm/_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
NSD,
NSD_OUTPUT_BICEP_PREFIX,
VNF,
SOURCE_ACR_REGEX
SOURCE_ACR_REGEX,
)

DESCRIPTION_MAP: Dict[str, str] = {
Expand Down Expand Up @@ -229,7 +229,7 @@ def network_function_name(self) -> str:
@property
def acr_manifest_name(self) -> str:
"""Return the ACR manifest name from the NFD name."""
sanitised_nf_name = self.network_function_name.lower().replace('_', '-')
sanitised_nf_name = self.network_function_name.lower().replace("_", "-")
return (
f"{sanitised_nf_name}-nsd-acr-manifest-{self.nsd_version.replace('.', '-')}"
)
Expand Down Expand Up @@ -329,10 +329,10 @@ def sa_manifest_name(self) -> str:
return f"{sanitized_nf_name}-sa-manifest-{self.version.replace('.', '-')}"

@property
def build_output_folder_name(self) -> str:
def output_directory_for_build(self) -> Path:
"""Return the local folder for generating the bicep template to."""
arm_template_path = self.arm_template.file_path
return f"{NF_DEFINITION_OUTPUT_BICEP_PREFIX}{Path(str(arm_template_path)).stem}"
arm_template_name = Path(self.arm_template.file_path).stem
return Path(f"{NF_DEFINITION_OUTPUT_BICEP_PREFIX}{arm_template_name}")


@dataclass
Expand Down Expand Up @@ -362,9 +362,9 @@ def __post_init__(self):
self.helm_packages[package_index] = HelmPackageConfig(**dict(package))

@property
def build_output_folder_name(self) -> str:
"""Return the local folder for generating the bicep template to."""
return f"{NF_DEFINITION_OUTPUT_BICEP_PREFIX}{self.nf_name}"
def output_directory_for_build(self) -> Path:
"""Return the directory the build command will writes its output to"""
return Path(f"{NF_DEFINITION_OUTPUT_BICEP_PREFIX}{self.nf_name}")

def validate(self):
"""Validate the CNF config
Expand All @@ -379,7 +379,9 @@ def validate(self):
if not source_registry_match or len(source_registry_match.groups()) < 2:
raise ValidationError(
"CNF config has an invalid source registry ID. Please run `az aosm "
"nfd generate-config` to see the valid formats.")
"nfd generate-config` to see the valid formats."
)


def get_configuration(
configuration_type: str, config_as_dict: Optional[Dict[Any, Any]] = None
Expand Down
17 changes: 9 additions & 8 deletions src/aosm/azext_aosm/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import os
import shutil
from dataclasses import asdict
from pathlib import Path
from typing import Optional

from azure.cli.core.azclierror import (
Expand Down Expand Up @@ -116,15 +117,15 @@ def _generate_nfd(
"Generate NFD called for unrecognised definition_type. Only VNF and CNF"
" have been implemented."
)
if nfd_generator.bicep_path:
if nfd_generator.nfd_bicep_path:
carry_on = input(
f"The folder {os.path.dirname(nfd_generator.bicep_path)} already exists -"
f"The {nfd_generator.nfd_bicep_path.parent} directory already exists -"
" delete it and continue? (y/n)"
)
if carry_on != "y":
raise UnclassifiedUserFault("User aborted! ")
raise UnclassifiedUserFault("User aborted!")

shutil.rmtree(os.path.dirname(nfd_generator.bicep_path))
shutil.rmtree(nfd_generator.nfd_bicep_path.parent)
nfd_generator.generate_nfd()


Expand Down Expand Up @@ -177,7 +178,7 @@ def publish_definition(
parameters_json_file=parameters_json_file,
manifest_bicep_path=manifest_file,
manifest_parameters_json_file=manifest_parameters_json_file,
skip=skip
skip=skip,
)
elif definition_type == CNF:
deployer = DeployerViaArm(api_clients, config=config)
Expand All @@ -187,7 +188,7 @@ def publish_definition(
parameters_json_file=parameters_json_file,
manifest_bicep_path=manifest_file,
manifest_parameters_json_file=manifest_parameters_json_file,
skip=skip
skip=skip,
)
else:
raise ValueError(
Expand Down Expand Up @@ -253,7 +254,7 @@ def _generate_config(configuration_type: str, output_file: str = "input.json"):
config = get_configuration(configuration_type)
config_as_dict = json.dumps(asdict(config), indent=4)

if os.path.exists(output_file):
if Path(output_file).exists():
carry_on = input(
f"The file {output_file} already exists - do you want to overwrite it?"
" (y/n)"
Expand Down Expand Up @@ -369,7 +370,7 @@ def publish_design(
parameters_json_file=parameters_json_file,
manifest_bicep_path=manifest_file,
manifest_parameters_json_file=manifest_parameters_json_file,
skip=skip
skip=skip,
)


Expand Down
8 changes: 5 additions & 3 deletions src/aosm/azext_aosm/deploy/artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
"""A module to handle interacting with artifacts."""
import subprocess
from dataclasses import dataclass
from typing import Union, List
from typing import List, Union

from azure.cli.core.commands import LongRunningOperation
from azure.mgmt.containerregistry.models import ImportImageParameters, ImportSource
from azure.mgmt.containerregistry import ContainerRegistryManagementClient
from azure.mgmt.containerregistry.models import (ImportImageParameters,
ImportSource)
from azure.storage.blob import BlobClient, BlobType
from knack.log import get_logger
from knack.util import CLIError
Expand Down Expand Up @@ -157,7 +159,7 @@ def copy_image(
:param source_image: source image
:param target_registry_resource_group_name: target registry resource group name
:param target_registry_name: target registry name
:param target_tags: the list of tags to be applied to the imported image
:param target_tags: the list of tags to be applied to the imported image
should be of form: namepace/name:tag or name:tag
:param mode: mode for import
"""
Expand Down
4 changes: 1 addition & 3 deletions src/aosm/azext_aosm/deploy/deploy_with_arm.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"""Contains class for deploying generated definitions using ARM."""
import json
import os
import re
import shutil
import subprocess # noqa
import tempfile
Expand Down Expand Up @@ -37,7 +36,6 @@
VNF,
VNF_DEFINITION_BICEP_TEMPLATE_FILENAME,
VNF_MANIFEST_BICEP_TEMPLATE_FILENAME,
SOURCE_ACR_REGEX,
)
from azext_aosm.util.management_clients import ApiClients

Expand Down Expand Up @@ -120,7 +118,7 @@ def deploy_vnfd_from_bicep(
# User has not passed in a bicep template, so we are deploying the default
# one produced from building the NFDV using this CLI
bicep_path = os.path.join(
self.config.build_output_folder_name,
self.config.output_directory_for_build,
VNF_DEFINITION_BICEP_TEMPLATE_FILENAME,
)

Expand Down
Loading

0 comments on commit 0497ec7

Please sign in to comment.