From 4e78afdacf91f3852c7b45d153875968c0cfcb47 Mon Sep 17 00:00:00 2001 From: Asespinel <79876430+Asespinel@users.noreply.github.com> Date: Wed, 27 Mar 2024 22:22:32 -0500 Subject: [PATCH] feat: add quince compatibility for distro (#57) * feat!: added quince support * feat: added private packages compatibility for quince * fix: fixed unit tests errors * feat: added init file necessary for imports * chore: added docstrings for quality tests and renamed methods for better understanding * chore: erased unnecessary printing lines * fix: added new method to fix wrong path when installing distro with pip and git * chore: removed unnecessary code referring the 'private.txt' file * fix: updated minimun tutor version to fix private packages not installing properly --- README.md | 7 ++- features/private_package_enabler.feature | 2 - .../steps/private_package_enabler_step.py | 9 --- setup.py | 4 +- tests/distro/packages/application/__init__.py | 0 .../application/package_mock_mother.py | 12 ++++ .../application/test_package_cloner.py | 10 ++++ .../test_private_package_definer.py | 10 ++++ .../commands/enable_private_packages.py | 57 +++++++++++++++++-- ...dx-dev-dockerfile-post-python-requirements | 21 ------- 10 files changed, 91 insertions(+), 41 deletions(-) create mode 100644 tests/distro/packages/application/__init__.py delete mode 100644 tutordistro/patches/openedx-dev-dockerfile-post-python-requirements diff --git a/README.md b/README.md index 0caa62a..b0e3445 100644 --- a/README.md +++ b/README.md @@ -39,8 +39,8 @@ This plugin works with some docker images. These are defined by default if you have different images that aren't based on these, you can have some problems. ```yaml -DOCKER_IMAGE_OPENEDX: "docker.io/ednxops/distro-edunext-edxapp:palma" -DOCKER_IMAGE_OPENEDX_DEV: "docker.io/ednxops/distro-edunext-edxapp-dev:palma" +DOCKER_IMAGE_OPENEDX: "docker.io/ednxops/distro-edunext-edxapp:quince" +DOCKER_IMAGE_OPENEDX_DEV: "docker.io/ednxops/distro-edunext-edxapp-dev:quince" ``` Also, you need an edx-platform version distro compatible. @@ -52,6 +52,7 @@ Also, you need an edx-platform version distro compatible. | nutmeg | nuez | v14 | | olive | olmo | v15 | | palm | palma | v16 | +| quince | quince | v17 | :warning: **NOTE**: From Olmo version Distro has not defaulted packages. Now it is necessary to add the packages you want in ``config.yml`` file. See [How to add a new package](./docs/how_to_add_new_packages.rst) @@ -59,7 +60,7 @@ You can find distro releases on https://github.com/edunext/edunext-platform. ```yaml EDX_PLATFORM_REPOSITORY: "https://github.com/eduNEXT/edunext-platform.git" -EDX_PLATFORM_VERSION: "ednx-release/palma.master" +EDX_PLATFORM_VERSION: "ednx-release/quince.master" ``` # Packages diff --git a/features/private_package_enabler.feature b/features/private_package_enabler.feature index 353d4e0..52da0b0 100644 --- a/features/private_package_enabler.feature +++ b/features/private_package_enabler.feature @@ -6,7 +6,6 @@ Feature: Enable private packages And There is a private package When I write the command tutor distro enable-private-packages Then Packages will be cloned into requirements folder - And Packages will be present in the file private.txt @fixture.behave.tutor_root Scenario: Add private package when the package has already been cloned @@ -16,4 +15,3 @@ Feature: Enable private packages And Private package has already been cloned When I write the command tutor distro enable-private-packages and press yes Then Packages will be cloned again into requirements folder - And Packages will be present in the file private.txt diff --git a/features/steps/private_package_enabler_step.py b/features/steps/private_package_enabler_step.py index 2c61f46..714829c 100644 --- a/features/steps/private_package_enabler_step.py +++ b/features/steps/private_package_enabler_step.py @@ -23,15 +23,6 @@ def step_impl(context): # pylint: disable=function-redefined,missing-function-d assert os.path.exists(path) -@then("Packages will be present in the file private.txt") -def step_impl(context): # pylint: disable=function-redefined,missing-function-docstring - distro_packages = get_private_distro_packages(context.scenario.config) - private_file = f"{context.scenario.tutor_root}/env/build/openedx/requirements/private.txt" - with open(private_file, mode="r", encoding="utf-8") as private_requirements_file: - for package in distro_packages.values(): - assert package["name"] in private_requirements_file.read() - - @given("There is a private package") def step_impl(context): # pylint: disable=function-redefined,missing-function-docstring eox_test = { diff --git a/setup.py b/setup.py index e182f90..7e381b6 100644 --- a/setup.py +++ b/setup.py @@ -36,10 +36,10 @@ def load_about(): author="eduNEXT", description="edunext-distro plugin for Tutor", long_description=load_readme(), - packages=find_packages(exclude=["tests*"]), + packages=find_packages(), include_package_data=True, python_requires=">=3.8", - install_requires=["tutor>=16.0.0, <17", "click", "schema"], + install_requires=["tutor>=17.0.3, <18", "click", "schema"], extras_require={ "test": ["behave", "pytest", "pylint", "pytest-mock", "pycodestyle", "isort", "schema"] }, diff --git a/tests/distro/packages/application/__init__.py b/tests/distro/packages/application/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/distro/packages/application/package_mock_mother.py b/tests/distro/packages/application/package_mock_mother.py index 40b4b33..a19a655 100644 --- a/tests/distro/packages/application/package_mock_mother.py +++ b/tests/distro/packages/application/package_mock_mother.py @@ -1,4 +1,13 @@ +""" +Package mocker for tests. +""" + + class PackageMockMother: + """ + Defines a simple package using a name, a domain, a verison and extra commands. + """ + def __init__(self) -> None: self._name = "eox-tests" self._domain = "github.com" @@ -12,6 +21,9 @@ def create( version: str = None, extra: dict = None ): + """ + Setter method for package mocker + """ package = { "name": name if name else self._name, "domain": domain if domain else self._domain, diff --git a/tests/distro/packages/application/test_package_cloner.py b/tests/distro/packages/application/test_package_cloner.py index 15fff5e..73e54a6 100644 --- a/tests/distro/packages/application/test_package_cloner.py +++ b/tests/distro/packages/application/test_package_cloner.py @@ -1,3 +1,7 @@ +""" +Test the package cloner scenarios. +""" + import pytest from tests.distro.packages.application.package_mock_mother import PackageMockMother @@ -7,6 +11,9 @@ def test_should_clone_one_repository_in_a_path(): + """ + Normal repo cloning behaviour test. + """ # Given mocker = PackageMockMother() name, domain, version, extra = mocker.create() @@ -22,6 +29,9 @@ def test_should_clone_one_repository_in_a_path(): def test_should_fail_when_the_repos_has_already_been_cloned(): + """ + Error repo cloning behaviour test. + """ # Given mocker = PackageMockMother() name, domain, version, extra = mocker.create() diff --git a/tests/distro/packages/application/test_private_package_definer.py b/tests/distro/packages/application/test_private_package_definer.py index 1423457..4d1f6e0 100644 --- a/tests/distro/packages/application/test_private_package_definer.py +++ b/tests/distro/packages/application/test_private_package_definer.py @@ -1,3 +1,7 @@ +""" +Test the private package definer scenarios. +""" + import pytest from tests.distro.packages.application.package_mock_mother import PackageMockMother @@ -7,6 +11,9 @@ def test_should_define_a_private_package_if_it_has_already_been_cloned(): + """ + Defines the package as private if it has been cloned. + """ # Given mocker = PackageMockMother() name, *_ = mocker.create() @@ -23,6 +30,9 @@ def test_should_define_a_private_package_if_it_has_already_been_cloned(): def test_should_fail_if_package_has_not_been_cloned_yet(): + """ + Should not define the package as private since it has not been cloned. + """ # Given mocker = PackageMockMother() name, *_ = mocker.create() diff --git a/tutordistro/commands/enable_private_packages.py b/tutordistro/commands/enable_private_packages.py index 4383e59..f37a4e4 100644 --- a/tutordistro/commands/enable_private_packages.py +++ b/tutordistro/commands/enable_private_packages.py @@ -8,7 +8,6 @@ from tutor import config as tutor_config from tutordistro.distro.packages.application.package_cloner import PackageCloner -from tutordistro.distro.packages.application.private_package_definer import PrivatePackageDefiner from tutordistro.distro.packages.infrastructure.package_git_repository import PackageGitRepository from tutordistro.utils.packages import get_private_distro_packages @@ -30,7 +29,6 @@ def enable_private_packages(): repository = PackageGitRepository() cloner = PackageCloner(repository=repository) - definer = PrivatePackageDefiner(repository=repository) private_packages = get_private_distro_packages(config) requirements_directory = f"{directory}/env/build/openedx/requirements/" @@ -47,7 +45,58 @@ def enable_private_packages(): }, path=requirements_directory ) - definer(name=package["name"], - file_path=f"{requirements_directory}private.txt") + + # Run tutor mounts add command for the package + subprocess.check_output(f"tutor mounts add {requirements_directory}{package['name']}", shell=True) + hook = f'hooks.Filters.MOUNTED_DIRECTORIES.add_item(("openedx", "{package["name"]}"))' + + hook_writer(hook_to_append=hook) + subprocess.check_output("tutor config save", shell=True) except Exception as error: # pylint: disable=broad-exception-caught click.echo(error) + + +def get_distro_location(): + """ + Function to get the right distro path + """ + + try: + result = subprocess.run(['pip', 'show', 'tutor-contrib-edunext-distro'], + capture_output=True, text=True, check=True) + + # Check if the command was successful + if result.returncode == 0: + # Split the output into lines + lines = result.stdout.splitlines() + + # Loop through each line to find the Location + for line in lines: + if line.startswith('Location:'): + # Extract the location path + location_path = line.split(':', 1)[1].strip() + "/tutordistro/plugin.py" + return location_path + except subprocess.CalledProcessError as e: + # Print error message if command failed + print("Error running pip show distro:", e.stderr) + + # Return a default value if the location is not found or an error occurs + return None + + +def hook_writer(hook_to_append): + """ + Function to write the corresponding hooks depending on the private packages. + """ + file_path = get_distro_location() + with open(file_path, 'a+', encoding='utf-8') as my_file: # Open file in append and read mode + + my_file.seek(0) # Move the cursor to the beginning of the file + existing_lines = my_file.readlines() + package_name = hook_to_append.split('"')[3] # Extract package name from hook_to_append + + # Check if package name already exists in the file + if any(package_name in line for line in existing_lines): + print(f"Package '{package_name}' already present in the file.") + else: + my_file.write(hook_to_append + "\n") diff --git a/tutordistro/patches/openedx-dev-dockerfile-post-python-requirements b/tutordistro/patches/openedx-dev-dockerfile-post-python-requirements deleted file mode 100644 index fe07ae9..0000000 --- a/tutordistro/patches/openedx-dev-dockerfile-post-python-requirements +++ /dev/null @@ -1,21 +0,0 @@ -{%- for pkg in iter_values_named(suffix="_DPKG") %} -{%- if loop.first -%} -RUN mkdir -p /openedx/extra_deps/ -{% endif %} -{%- if pkg != 'None' -%} -{%- if pkg["private"] == false %} -# Install {{ pkg["name"] }} -{%- endif %} -{%+ if pkg["index"] and pkg["index"] == "pip" -%} -RUN pip install {{ pkg["name"] }}=={{ pkg["version"] }} -{%- elif not pkg["private"] -%} -RUN mkdir -p /openedx/extra_deps/{{ pkg["name"] }} && \ - git clone -b {{ pkg["version"] }} {{ pkg["protocol"] }}://{{ pkg["domain"] }}/{{ pkg["path"] }}/{{ pkg["repo"] }}.git /openedx/extra_deps/{{ pkg["name"] }} -RUN pip install -e /openedx/extra_deps/{{ pkg["name"] }} -{%- elif pkg["private"] -%} -RUN mv /openedx/requirements/{{ pkg["name"] }} /openedx/extra_deps/{{ pkg["name"] }} -RUN pip install -e /openedx/extra_deps/{{ pkg["name"] }} -{%- endif -%} -{%- endif -%} -{% endfor %} -