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

feat: add checks for the appropriate splunktaucclib version #1363

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
60 changes: 53 additions & 7 deletions splunk_add_on_ucc_framework/install_python_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import subprocess
import sys
from pathlib import Path

from packaging.version import Version
artemrys marked this conversation as resolved.
Show resolved Hide resolved
from typing import List, Optional, Set, Iterable, Dict
from splunk_add_on_ucc_framework.global_config import OSDependentLibraryConfig

Expand All @@ -34,6 +36,10 @@ class CouldNotInstallRequirements(Exception):
pass


class InvalidArguments(Exception):
pass


def _subprocess_call(
command: str,
command_desc: Optional[str] = None,
Expand All @@ -55,6 +61,24 @@ def _subprocess_call(
raise e


def _subprocess_run(
Copy link
Member

Choose a reason for hiding this comment

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

This looks very similar to _subprocess_call. I would suggest refactoring _subprocess_call into returning full response and using it everywhere.

command: str,
command_desc: Optional[str] = None,
env: Optional[Dict[str, str]] = None,
) -> "subprocess.CompletedProcess[bytes]":
command_desc = command_desc or command
try:
logger.info(f"Executing: {command}")
process_result = subprocess.run(
command, shell=True, env=env, capture_output=True
)
return process_result

except OSError as e:
logger.error(f"Execution ({command_desc}) failed due to {e}")
raise e


def _pip_install(installer: str, command: str, command_desc: str) -> None:
cmd = f"{installer} -m pip install {command}"
try:
Expand All @@ -66,18 +90,38 @@ def _pip_install(installer: str, command: str, command_desc: str) -> None:


def _pip_is_lib_installed(
installer: str, target: str, libname: str, version: Optional[str] = None
installer: str,
target: str,
libname: str,
version: Optional[str] = None,
allow_higher_version: bool = False,
) -> bool:
if not version and allow_higher_version:
raise InvalidArguments(
"Parameter 'allow_higher_version' can not be set to True if 'version' parameter is not provided"
)

lib_installed_cmd = f"{installer} -m pip show --version {libname}"
lib_version_match_cmd = f'{lib_installed_cmd} | grep "Version: {version}"'

cmd = lib_version_match_cmd if version else lib_installed_cmd
if version and allow_higher_version:
cmd = f'{lib_installed_cmd} | grep "Version"'
elif version and not allow_higher_version:
cmd = f'{lib_installed_cmd} | grep "Version: {version}"'
else:
cmd = lib_installed_cmd

try:
my_env = os.environ.copy()
my_env["PYTHONPATH"] = target
return_code = _subprocess_call(command=cmd, env=my_env)
return return_code == 0
if allow_higher_version:
result = _subprocess_run(command=cmd, env=my_env)
if result.returncode != 0:
return False
result_version = result.stdout.decode("utf-8").split("Version:")[1].strip()
return Version(result_version) >= Version(version)
else:
return_code = _subprocess_call(command=cmd, env=my_env)
return return_code == 0
except OSError as e:
raise CouldNotInstallRequirements from e

Expand Down Expand Up @@ -116,10 +160,12 @@ def install_python_libraries(
installer=python_binary_name,
target=ucc_lib_target,
libname="splunktaucclib",
version="6.4",
allow_higher_version=True,
):
raise SplunktaucclibNotFound(
f"splunktaucclib is not found in {path_to_requirements_file}. "
f"Please add it there because this add-on has UI."
f"splunktaucclib is not found in {path_to_requirements_file} or has invalid version. "
f"Please add it there and check if it is at least version 6.4, because this add-on has UI."
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to separate this into 2 different exceptions / messages:

  • there is no splunktaucclib found but this add-ons has UI
  • splunktaucclib found but the developer needs to update it to the version we specify because of

I think it would be easier to understand why build does not actually work.

)

cleanup_libraries = install_os_dependent_libraries(
Expand Down
20 changes: 17 additions & 3 deletions tests/unit/test_install_python_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ def test_install_libraries_when_subprocess_returns_non_zero_codes(
)


@mock.patch("subprocess.call", autospec=True)
def test_install_python_libraries(mock_subprocess_call, tmp_path):
mock_subprocess_call.return_value = 0
def test_install_python_libraries(tmp_path):
tmp_ucc_lib_target = tmp_path / "ucc-lib-target"
tmp_ucc_lib_target.mkdir()
tmp_lib_path = tmp_path / "lib"
Expand Down Expand Up @@ -200,6 +198,22 @@ def test_install_libraries_when_no_splunktaucclib_is_present_but_has_ui(tmp_path
)


def test_install_libraries_when_wrong_splunktaucclib_is_present_but_has_ui(tmp_path):
tmp_ucc_lib_target = tmp_path / "ucc-lib-target"
tmp_lib_path = tmp_path / "lib"
tmp_lib_path.mkdir()
tmp_lib_reqs_file = tmp_lib_path / "requirements.txt"
tmp_lib_reqs_file.write_text("splunktaucclib==6.3\n")

with pytest.raises(SplunktaucclibNotFound):
install_python_libraries(
str(tmp_path),
str(tmp_ucc_lib_target),
python_binary_name="python3",
includes_ui=True,
)


def test_remove_package_from_installed_path(tmp_path):
tmp_lib_path = tmp_path / "lib"
tmp_lib_path.mkdir()
Expand Down
Loading