Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pathlib.Path for path functionality #217

Merged
merged 6 commits into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions gitopscli/appconfig_api/app_tenant_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import os
from dataclasses import dataclass, field
from pathlib import Path
from typing import Any

from gitopscli.git_api import GitRepo
Expand Down Expand Up @@ -99,16 +100,12 @@ def __generate_config_from_tenant_repo(

def __get_all_tenant_applications_dirs(tenant_repo: GitRepo) -> set[str]:
repo_dir = tenant_repo.get_full_file_path(".")
return {
name
for name in os.listdir(repo_dir)
if os.path.isdir(os.path.join(repo_dir, name)) and not name.startswith(".")
}
return {name for name in os.listdir(repo_dir) if (Path(repo_dir) / name).is_dir() and not name.startswith(".")}


def __get_custom_config(appname: str, tenant_config_git_repo: GitRepo) -> Any:
custom_config_path = tenant_config_git_repo.get_full_file_path(f"{appname}/.config.yaml")
if os.path.exists(custom_config_path):
if Path(custom_config_path).exists():
return yaml_file_load(custom_config_path)
return {}

Expand Down
6 changes: 3 additions & 3 deletions gitopscli/commands/create_preview.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import logging
import os
import shutil
from collections.abc import Callable
from dataclasses import dataclass
from pathlib import Path
from typing import Any

from gitopscli.git_api import GitApiConfig, GitRepo, GitRepoApi, GitRepoApiFactory
Expand Down Expand Up @@ -128,13 +128,13 @@ def __create_preview_from_template_if_not_existing(
) -> bool:
preview_namespace = gitops_config.get_preview_namespace(self.__args.preview_id)
full_preview_folder_path = target_git_repo.get_full_file_path(preview_namespace)
preview_env_already_exist = os.path.isdir(full_preview_folder_path)
preview_env_already_exist = Path(full_preview_folder_path).is_dir()
if preview_env_already_exist:
logging.info("Use existing folder for preview: %s", preview_namespace)
return False
logging.info("Create new folder for preview: %s", preview_namespace)
full_preview_template_folder_path = template_git_repo.get_full_file_path(gitops_config.preview_template_path)
if not os.path.isdir(full_preview_template_folder_path):
if not Path(full_preview_template_folder_path).is_dir():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could extract those checks into a separate dependency to prevent the need to continue to do this ugly monkey patching. Something like path_checker.is_dir(full_preview_template_folder_path)

raise GitOpsException(f"The preview template folder does not exist: {gitops_config.preview_template_path}")
logging.info("Using the preview template folder: %s", gitops_config.preview_template_path)
shutil.copytree(full_preview_template_folder_path, full_preview_folder_path)
Expand Down
4 changes: 2 additions & 2 deletions gitopscli/commands/delete_preview.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import os
import shutil
from dataclasses import dataclass
from pathlib import Path

from gitopscli.git_api import GitApiConfig, GitRepo, GitRepoApi, GitRepoApiFactory
from gitopscli.gitops_config import GitOpsConfig
Expand Down Expand Up @@ -79,7 +79,7 @@ def __commit_and_push(self, git_repo: GitRepo, message: str) -> None:
@staticmethod
def __delete_folder_if_exists(git_repo: GitRepo, folder_name: str) -> bool:
folder_full_path = git_repo.get_full_file_path(folder_name)
if not os.path.exists(folder_full_path):
if not Path(folder_full_path).exists():
return False
shutil.rmtree(folder_full_path, ignore_errors=True)
return True
12 changes: 6 additions & 6 deletions gitopscli/git_api/git_repo.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import locale
import logging
import os
from pathlib import Path
from types import TracebackType
from typing import Literal

Expand Down Expand Up @@ -35,7 +35,7 @@ def finalize(self) -> None:

def get_full_file_path(self, relative_path: str) -> str:
repo = self.__get_repo()
return os.path.join(str(repo.working_dir), relative_path)
return str(Path(repo.working_dir) / relative_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could directly return Path object here. It's re-wrapped everywhere anyways


def get_clone_url(self) -> str:
return self.__api.get_clone_url()
Expand Down Expand Up @@ -132,10 +132,10 @@ def __get_repo(self) -> Repo:
raise GitOpsException("Repository not cloned yet!")

def __create_credentials_file(self, username: str, password: str) -> str:
file_path = f"{self.__tmp_dir}/credentials.sh"
with open(file_path, "w", encoding=locale.getpreferredencoding(do_setlocale=False)) as text_file:
file_path = Path(f"{self.__tmp_dir}/credentials.sh")
with file_path.open("w", encoding=locale.getpreferredencoding(do_setlocale=False)) as text_file:
text_file.write("#!/bin/sh\n")
text_file.write(f"echo username='{username}'\n")
text_file.write(f"echo password='{password}'\n")
os.chmod(file_path, 0o700)
return file_path
file_path.chmod(0o700)
return str(file_path)
4 changes: 2 additions & 2 deletions gitopscli/io_api/tmp_dir.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import os
import shutil
import uuid
from pathlib import Path


def create_tmp_dir() -> str:
tmp_dir = f"/tmp/gitopscli/{uuid.uuid4()}" # noqa: S108
os.makedirs(tmp_dir)
Path(tmp_dir).mkdir(parents=True)
return tmp_dir


Expand Down
5 changes: 3 additions & 2 deletions gitopscli/io_api/yaml_util.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import locale
from io import StringIO
from pathlib import Path
from typing import Any

from jsonpath_ng.exceptions import JSONPathError
Expand All @@ -15,15 +16,15 @@ class YAMLException(Exception): # noqa: N818


def yaml_file_load(file_path: str) -> Any:
with open(file_path, encoding=locale.getpreferredencoding(do_setlocale=False)) as stream:
with Path(file_path).open(encoding=locale.getpreferredencoding(do_setlocale=False)) as stream:
try:
return YAML_INSTANCE.load(stream)
except YAMLError as ex:
raise YAMLException(f"Error parsing YAML file: {file_path}") from ex


def yaml_file_dump(yaml: Any, file_path: str) -> None:
with open(file_path, "w+", encoding=locale.getpreferredencoding(do_setlocale=False)) as stream:
with Path(file_path).open("w+", encoding=locale.getpreferredencoding(do_setlocale=False)) as stream:
YAML_INSTANCE.dump(yaml, stream)


Expand Down
11 changes: 0 additions & 11 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,3 @@ ignore = [
"ANN", # https://docs.astral.sh/ruff/rules/#flake8-annotations-ann
"PT009" # https://docs.astral.sh/ruff/rules/pytest-unittest-assertion/
]
# the following exclusions have been introduced to prevent huge changes
# feel free to remove them and fix the code
"gitopscli/appconfig_api/app_tenant_config.py" = ["PTH110", "PTH112", "PTH118"]
"gitopscli/commands/create_preview.py" = ["PTH112"]
"gitopscli/commands/delete_preview.py" = ["PTH110"]
"gitopscli/git_api/git_repo.py" = ["PTH101", "PTH118", "PTH123"]
"gitopscli/io_api/tmp_dir.py" = ["PTH103"]
"gitopscli/io_api/yaml_util.py" = ["PTH123"]
"tests/git_api/test_git_repo.py" = ["PTH101", "PTH103", "PTH110", "PTH123"]
"tests/io_api/test_tmp_dir.py" = ["PTH103", "PTH112"]
"tests/io_api/test_yaml_util.py" = ["PTH103", "PTH123"]
90 changes: 52 additions & 38 deletions tests/commands/test_create_preview.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import os
import shutil
import unittest
from pathlib import Path
from unittest.mock import Mock, call

from gitopscli.commands.create_preview import CreatePreviewCommand, load_gitops_config
Expand Down Expand Up @@ -45,8 +45,9 @@ class CreatePreviewCommandTest(MockMixin, unittest.TestCase):
def setUp(self):
self.init_mock_manager(CreatePreviewCommand)

self.os_mock = self.monkey_patch(os)
self.os_mock.path.isdir.return_value = True
self.path_mock = self.monkey_patch(Path)
self.path_mock.return_value = self.path_mock
self.path_mock.is_dir.return_value = True

self.shutil_mock = self.monkey_patch(shutil)
self.shutil_mock.copytree.return_value = None
Expand Down Expand Up @@ -126,10 +127,10 @@ def git_repo_constructor_mock(git_repo_api: GitRepoApi) -> GitRepo:
self.seal_mocks()

def test_create_new_preview(self):
self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": False, # doesn't exist yet -> expect create
"/tmp/template-repo/.preview-templates/my-app": True,
}[path]
self.path_mock.is_dir.side_effect = [
False, # /tmp/target-repo/my-app-685912d3-preview, doesn't exist yet -> expect create
True, # /tmp/template-repo/.preview-templates/my-app
]

deployment_created_callback = Mock(return_value=None)

Expand Down Expand Up @@ -161,10 +162,12 @@ def test_create_new_preview(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Create new folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path(".preview-templates/my-app"),
call.os.path.isdir("/tmp/template-repo/.preview-templates/my-app"),
call.Path("/tmp/template-repo/.preview-templates/my-app"),
call.Path.is_dir(),
call.logging.info("Using the preview template folder: %s", ".preview-templates/my-app"),
call.shutil.copytree(
"/tmp/template-repo/.preview-templates/my-app", "/tmp/target-repo/my-app-685912d3-preview"
Expand Down Expand Up @@ -229,10 +232,10 @@ def test_create_new_preview_from_same_template_target_repo(self):
replacements=gitops_config.replacements,
)

self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": False, # doesn't exist yet -> expect create
"/tmp/target-repo/.preview-templates/my-app": True,
}[path]
self.path_mock.is_dir.side_effect = [
False, # /tmp/target-repo/my-app-685912d3-preview, doesn't exist yet -> expect create
True, # /tmp/target-repo/.preview-templates/my-app
]

deployment_created_callback = Mock(return_value=None)

Expand Down Expand Up @@ -263,10 +266,12 @@ def test_create_new_preview_from_same_template_target_repo(self):
call.GitRepo(self.target_git_repo_api_mock),
call.GitRepo.clone(None),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Create new folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path(".preview-templates/my-app"),
call.os.path.isdir("/tmp/target-repo/.preview-templates/my-app"),
call.Path("/tmp/target-repo/.preview-templates/my-app"),
call.Path.is_dir(),
call.logging.info("Using the preview template folder: %s", ".preview-templates/my-app"),
call.shutil.copytree(
"/tmp/target-repo/.preview-templates/my-app", "/tmp/target-repo/my-app-685912d3-preview"
Expand Down Expand Up @@ -311,9 +316,9 @@ def test_create_new_preview_from_same_template_target_repo(self):
]

def test_update_existing_preview(self):
self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": True, # already exists -> expect update
}[path]
self.path_mock.is_dir.side_effect = [
True, # /tmp/target-repo/my-app-685912d3-preview, already exists -> expect update
]

deployment_updated_callback = Mock(return_value=None)

Expand All @@ -337,7 +342,8 @@ def test_update_existing_preview(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview/Chart.yaml"),
call.update_yaml_file(
Expand Down Expand Up @@ -379,9 +385,9 @@ def test_update_existing_preview(self):
]

def test_preview_already_up_to_date(self):
self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": True, # already exists -> expect update
}[path]
self.path_mock.is_dir.side_effect = [
True, # /tmp/target-repo/my-app-685912d3-preview, already exists -> expect update
]

self.update_yaml_file_mock.return_value = False # nothing updated -> expect already up to date

Expand All @@ -407,7 +413,8 @@ def test_preview_already_up_to_date(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview/Chart.yaml"),
call.update_yaml_file(
Expand Down Expand Up @@ -437,10 +444,10 @@ def test_preview_already_up_to_date(self):
]

def test_create_preview_for_unknown_template(self):
self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": False,
"/tmp/template-repo/.preview-templates/my-app": False, # preview template missing -> expect error
}[path]
self.path_mock.is_dir.side_effect = [
False, # /tmp/target-repo/my-app-685912d3-preview
False, # /tmp/template-repo/.preview-templates/my-app, preview template missing -> expect error
]

try:
CreatePreviewCommand(ARGS).execute()
Expand All @@ -458,10 +465,12 @@ def test_create_preview_for_unknown_template(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Create new folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path(".preview-templates/my-app"),
call.os.path.isdir("/tmp/template-repo/.preview-templates/my-app"),
call.Path("/tmp/template-repo/.preview-templates/my-app"),
call.Path.is_dir(),
]

def test_create_preview_values_yaml_not_found(self):
Expand All @@ -483,7 +492,8 @@ def test_create_preview_values_yaml_not_found(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview/Chart.yaml"),
call.update_yaml_file(
Expand All @@ -510,7 +520,8 @@ def test_create_preview_values_yaml_parse_error(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview/Chart.yaml"),
call.update_yaml_file(
Expand All @@ -537,7 +548,8 @@ def test_create_preview_with_invalid_replacement_path(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview/Chart.yaml"),
call.update_yaml_file(
Expand All @@ -546,10 +558,10 @@ def test_create_preview_with_invalid_replacement_path(self):
]

def test_create_new_preview_invalid_chart_template(self):
self.os_mock.path.isdir.side_effect = lambda path: {
"/tmp/target-repo/my-app-685912d3-preview": False, # doesn't exist yet -> expect create
"/tmp/template-repo/.preview-templates/my-app": True,
}[path]
self.path_mock.is_dir.side_effect = [
False, # /tmp/target-repo/my-app-685912d3-preview doesn't exist yet -> expect create
True, # /tmp/template-repo/.preview-templates/my-app
]

self.update_yaml_file_mock.side_effect = KeyError()

Expand All @@ -569,10 +581,12 @@ def test_create_new_preview_invalid_chart_template(self):
call.GitRepo(self.template_git_repo_api_mock),
call.GitRepo.clone("template-branch"),
call.GitRepo.get_full_file_path("my-app-685912d3-preview"),
call.os.path.isdir("/tmp/target-repo/my-app-685912d3-preview"),
call.Path("/tmp/target-repo/my-app-685912d3-preview"),
call.Path.is_dir(),
call.logging.info("Create new folder for preview: %s", "my-app-685912d3-preview"),
call.GitRepo.get_full_file_path(".preview-templates/my-app"),
call.os.path.isdir("/tmp/template-repo/.preview-templates/my-app"),
call.Path("/tmp/template-repo/.preview-templates/my-app"),
call.Path.is_dir(),
call.logging.info("Using the preview template folder: %s", ".preview-templates/my-app"),
call.shutil.copytree(
"/tmp/template-repo/.preview-templates/my-app", "/tmp/target-repo/my-app-685912d3-preview"
Expand Down
Loading