From 47e09277cd5715a0513f7c80952b5470bf623b8c Mon Sep 17 00:00:00 2001 From: Martynas Asipauskas Date: Thu, 14 Nov 2024 15:58:28 +0000 Subject: [PATCH] Revert client changes (#4050) --- .../python-client-release-to-pypi.yml | 4 +- .github/workflows/python-client.yml | 7 +- .github/workflows/python-tests/action.yml | 4 +- .gitignore | 1 - build/python-client/Dockerfile | 4 +- .../python/armada_client/gen/event_typings.py | 27 ++-- client/python/armada_client/proto/.gitkeep | 0 client/python/docs/source/conf.py | 2 + client/python/pyproject.toml | 13 +- client/python/setup.py | 124 ------------------ client/python/tox.ini | 7 +- magefiles/tests.go | 6 +- scripts/build-python-client.sh | 35 ++++- 13 files changed, 73 insertions(+), 161 deletions(-) delete mode 100644 client/python/armada_client/proto/.gitkeep delete mode 100644 client/python/setup.py diff --git a/.github/workflows/python-client-release-to-pypi.yml b/.github/workflows/python-client-release-to-pypi.yml index 7c4877b2f6c..33866ffd5c9 100644 --- a/.github/workflows/python-client-release-to-pypi.yml +++ b/.github/workflows/python-client-release-to-pypi.yml @@ -19,8 +19,8 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: ./.github/workflows/python-tests with: - python-version: '3.9' - tox-env: 'py39' + python-version: '3.8' + tox-env: 'py38' path: 'client/python' github-token: ${{secrets.GITHUB_TOKEN}} - name: Publish package to PyPI diff --git a/.github/workflows/python-client.yml b/.github/workflows/python-client.yml index a9f5cfd9155..8541394a811 100644 --- a/.github/workflows/python-client.yml +++ b/.github/workflows/python-client.yml @@ -34,16 +34,13 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - python: [ '3.9', '3.10', '3.11', '3.12' ] + python: [ '3.8', '3.9', '3.10' ] include: + - tox-env: 'py38' - tox-env: 'py39' python: '3.9' - tox-env: 'py310' python: '3.10' - - tox-env: 'py311' - python: '3.11' - - tox-env: 'py312' - python: '3.12' steps: - uses: actions/checkout@v4 - name: Setup Go diff --git a/.github/workflows/python-tests/action.yml b/.github/workflows/python-tests/action.yml index 5d2b34a57a2..3f261dd5675 100644 --- a/.github/workflows/python-tests/action.yml +++ b/.github/workflows/python-tests/action.yml @@ -21,7 +21,7 @@ runs: with: python-version: ${{ inputs.python-version }} # Tox to run tests; build to build the wheel after tests pass - - run: pip install tox==4.17.0 build twine setuptools + - run: pip install tox==3.27.1 build twine shell: bash - name: Install Protoc uses: arduino/setup-protoc@v3 @@ -45,7 +45,7 @@ runs: working-directory: ${{ inputs.path }} - name: Build and verify wheel run: | - python -m build --sdist + python -m build --wheel twine check dist/* shell: bash working-directory: ${{ inputs.path }} diff --git a/.gitignore b/.gitignore index 00c2b8adc71..73c5e2d5452 100644 --- a/.gitignore +++ b/.gitignore @@ -77,7 +77,6 @@ client/python/dist *_pb2.py *_pb2.pyi *_pb2_grpc.py -client/python/armada_client/proto/ client/python/armada_client/armada/ .tox proto-airflow diff --git a/build/python-client/Dockerfile b/build/python-client/Dockerfile index 719bf9abf41..10aa957944b 100644 --- a/build/python-client/Dockerfile +++ b/build/python-client/Dockerfile @@ -1,5 +1,5 @@ ARG PLATFORM=x86_64 -ARG BASE_IMAGE=python:3.9.20-bookworm +ARG BASE_IMAGE=python:3.8.18-bookworm FROM --platform=$PLATFORM ${BASE_IMAGE} @@ -7,7 +7,7 @@ RUN mkdir /proto COPY client/python/pyproject.toml /code/pyproject.toml -RUN pip install setuptools "/code[test]" +RUN pip install "/code[test]" # Creating folders, and files for a project: COPY client/python /code diff --git a/client/python/armada_client/gen/event_typings.py b/client/python/armada_client/gen/event_typings.py index ef2ff45201c..cf2692143f6 100644 --- a/client/python/armada_client/gen/event_typings.py +++ b/client/python/armada_client/gen/event_typings.py @@ -1,5 +1,3 @@ -import argparse -from pathlib import Path import sys from armada_client.armada import event_pb2, submit_pb2 @@ -65,7 +63,15 @@ def gen_file(states, classes, jobstates): return import_text, states_text, union_text, jobstates_text -def main(typings_file: Path): +def write_file(import_text, states_text, union_text, jobstates_text, file): + with open(f"{file}", "w", encoding="utf-8") as f: + f.write(import_text) + f.write(states_text) + f.write(jobstates_text) + f.write(union_text) + + +def main(): states = get_event_states() print("Done creating EventStates") @@ -78,16 +84,13 @@ def main(typings_file: Path): import_text, states_text, union_text, jobstates_text = gen_file( states, classes, jobstates ) - typings_file.write_text(import_text + states_text + jobstates_text + union_text) + write_file(import_text, states_text, union_text, jobstates_text, typings_file) if __name__ == "__main__": - parser = argparse.ArgumentParser() - parser.add_argument("typings_file", type=Path, help="Path to typings file") - - args = parser.parse_args() - print(f"{args}") - typings_file = args.typings_file or Path("armada_client") / "typings.py" - print(f"{typings_file}") - main(typings_file) + # get path to this files location + root = f"{sys.path[0]}/../../" + typings_file = f"{root}/armada_client/typings.py" + + main() sys.exit(0) diff --git a/client/python/armada_client/proto/.gitkeep b/client/python/armada_client/proto/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/client/python/docs/source/conf.py b/client/python/docs/source/conf.py index 294e0926f11..cffef73a2e1 100644 --- a/client/python/docs/source/conf.py +++ b/client/python/docs/source/conf.py @@ -13,6 +13,8 @@ import os import sys +sys.path.insert(0, os.path.abspath("../..")) + # -- Project information ----------------------------------------------------- diff --git a/client/python/pyproject.toml b/client/python/pyproject.toml index b6a89437820..5b0952245f3 100644 --- a/client/python/pyproject.toml +++ b/client/python/pyproject.toml @@ -1,10 +1,10 @@ [project] name = "armada_client" -version = "0.4.5" +version = "0.4.6" description = "Armada gRPC API python client" readme = "README.md" -requires-python = ">=3.9" -dependencies = ["grpcio-tools", "protobuf>3.20,<5.0"] +requires-python = ">=3.7" +dependencies = ["grpcio==1.66.1", "grpcio-tools==1.66.1", "mypy-protobuf>=3.2.0", "protobuf>=5.26.1,<6.0dev" ] license = { text = "Apache Software License" } authors = [{ name = "G-Research Open Source Software", email = "armada@armadaproject.io" }] @@ -12,10 +12,10 @@ authors = [{ name = "G-Research Open Source Software", email = "armada@armadapro format = ["black==23.7.0", "flake8==7.0.0", "pylint==2.17.5"] # note(JayF): sphinx-jekyll-builder was broken by sphinx-markdown-builder 0.6 -- so pin to 0.5.5 docs = ["sphinx==7.1.2", "sphinx-jekyll-builder==0.3.0", "sphinx-toolbox==3.2.0b1", "sphinx-markdown-builder==0.5.5"] -test = ["pytest==7.3.1", "pytest-cov", "pytest-asyncio==0.21.1"] +test = ["pytest==7.3.1", "coverage>=6.5.0", "pytest-asyncio==0.21.1"] [build-system] -requires = ["setuptools", "wheel", "grpcio-tools", "mypy-protobuf", "protobuf>3.20,<5.0"] +requires = ["setuptools"] build-backend = "setuptools.build_meta" [tool.mypy] @@ -39,5 +39,4 @@ omit = [ # py.typed is required for mypy to find type hints in the package # from: https://mypy.readthedocs.io/en/stable/installed_packages.html#making-pep-561-compatible-packages [tool.setuptools.package-data] -"*" = ["*.pyi", "py.typed", "proto/**/*.proto"] - +"*" = ["*.pyi", "py.typed"] diff --git a/client/python/setup.py b/client/python/setup.py deleted file mode 100644 index 9bdbb3e82bc..00000000000 --- a/client/python/setup.py +++ /dev/null @@ -1,124 +0,0 @@ -import os -from pathlib import Path -import shutil -import subprocess -import sys -from typing import Dict -from setuptools import setup -import importlib.resources -import re -from setuptools.command.build_py import build_py - - -def generate_grpc_bindings(build_lib: Path): - import grpc_tools.protoc - - proto_include = importlib.resources.path("grpc_tools", "_proto") - proto_files = [ - "google/api/annotations.proto", - "google/api/http.proto", - "github.com/gogo/protobuf/gogoproto/gogo.proto", - "k8s.io/api/core/v1/generated.proto", - "k8s.io/apimachinery/pkg/api/resource/generated.proto", - "k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto", - "k8s.io/apimachinery/pkg/runtime/generated.proto", - "k8s.io/apimachinery/pkg/runtime/schema/generated.proto", - "k8s.io/apimachinery/pkg/util/intstr/generated.proto", - "k8s.io/api/networking/v1/generated.proto", - "armada/event.proto", - "armada/submit.proto", - "armada/health.proto", - "armada/job.proto", - "armada/binoculars.proto", - ] - target_root = build_lib.absolute() / "armada_client" - - for proto_file in proto_files: - command = [ - f"-I{proto_include}", - f"-I{target_root / 'proto'}", - f"--python_out={target_root}", - f"--grpc_python_out={target_root}", - f"--mypy_out={target_root}", - str(target_root / "proto" / proto_file), - ] - if grpc_tools.protoc.main(command) != 0: - raise Exception(f"grpc_tools.protoc.main: {command} failed") - - shutil.rmtree(target_root / "github.com") - shutil.rmtree(target_root / "k8s.io") - - adjust_import_paths(target_root) - - -def adjust_import_paths(output_dir: Path): - replacements = { - r"from armada": "from armada_client.armada", - r"from github.com": "from armada_client.github.com", - r"from google.api": "from armada_client.google.api", - } - - for file in output_dir.glob("armada/*.py"): - replace_in_file(file, replacements) - for file in output_dir.glob("google/api/*.py"): - replace_in_file(file, replacements) - - replacements = { - r"from k8s.io": "from armada_client.k8s.io", - } - for file in output_dir.glob("../**/*.py"): - replace_in_file(file, replacements) - - replacements = { - r" k8s": " armada_client.k8s", - r"\[k8s": "[armada_client.k8s", - r"import k8s.io": "import armada_client.k8s.io", - } - for file in output_dir.glob("k8s/**/*.pyi"): - replace_in_file(file, replacements) - - -def replace_in_file(file: Path, replacements: Dict[str, str]): - """Replace patterns in a file based on the replacements dictionary.""" - - content = file.read_text() - for pattern, replacement in replacements.items(): - content = re.sub(pattern, replacement, content) - file.write_text(content) - - -def generate_typings(build_dir: Path): - typings = build_dir.absolute() / "armada_client" / "typings.py" - result = subprocess.run( - args=[ - sys.executable, - str(build_dir.absolute() / "armada_client" / "gen" / "event_typings.py"), - str(typings), - ], - env={"PYTHONPATH": str(build_dir.absolute())}, - capture_output=True, - ) - if result.returncode != 0: - print(result.stdout) - print(result.stderr) - result.check_returncode() - - -class BuildPackageProtos(build_py): - """ - Generate GRPC code before building the package. - """ - - def run(self): - super().run() - output_dir = Path(".") if self.editable_mode else Path(self.build_lib) - generate_grpc_bindings(output_dir) - generate_typings(output_dir) - - -setup( - cmdclass={ - "build_py": BuildPackageProtos, - "develop": BuildPackageProtos, - }, -) diff --git a/client/python/tox.ini b/client/python/tox.ini index 4e31d820a24..5eaaa29ceec 100644 --- a/client/python/tox.ini +++ b/client/python/tox.ini @@ -1,15 +1,16 @@ [tox] +isolated_build = true envlist = format + py38 py39 py310 - py311 - py312 [testenv] extras = test commands = - pytest --cov={envsitepackagesdir}/armada_client --cov-report=xml --cov-report=term tests/unit/ + coverage run -m pytest tests/unit/ + coverage xml [testenv:docs] extras = docs diff --git a/magefiles/tests.go b/magefiles/tests.go index 78653eab0dc..e4460c8eee0 100644 --- a/magefiles/tests.go +++ b/magefiles/tests.go @@ -169,10 +169,12 @@ func Teste2epython() error { "--workdir", "/code", "-e", "ARMADA_SERVER=server", "-e", "ARMADA_PORT=50051", + "--entrypoint", "python3", "--network", "kind", "armada-python-client-builder:latest", - "-c", - "pip install -e . && python3 -m pytest -v -s /code/tests/integration/test_no_auth.py", + "-m", "pytest", + "-v", "-s", + "/code/tests/integration/test_no_auth.py", } return dockerRun(args...) diff --git a/scripts/build-python-client.sh b/scripts/build-python-client.sh index 7004989e596..5fd23818146 100755 --- a/scripts/build-python-client.sh +++ b/scripts/build-python-client.sh @@ -5,4 +5,37 @@ mkdir -p proto/armada cp pkg/api/event.proto pkg/api/submit.proto pkg/api/health.proto pkg/api/job.proto pkg/api/binoculars/binoculars.proto proto/armada sed -i 's/\([^\/]\)pkg\/api/\1armada/g' proto/armada/*.proto -cp -rf proto/* client/python/armada_client/proto/ + +# generate python stubs +cd proto +python3 -m grpc_tools.protoc -I. --plugin=protoc-gen-mypy=$(which protoc-gen-mypy) --python_out=../client/python/armada_client --grpc_python_out=../client/python/armada_client --mypy_out=../client/python/armada_client \ + google/api/annotations.proto \ + google/api/http.proto \ + armada/event.proto armada/submit.proto armada/health.proto armada/job.proto armada/binoculars.proto \ + github.com/gogo/protobuf/gogoproto/gogo.proto \ + k8s.io/api/core/v1/generated.proto \ + k8s.io/apimachinery/pkg/api/resource/generated.proto \ + k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto \ + k8s.io/apimachinery/pkg/runtime/generated.proto \ + k8s.io/apimachinery/pkg/runtime/schema/generated.proto \ + k8s.io/apimachinery/pkg/util/intstr/generated.proto \ + k8s.io/api/networking/v1/generated.proto + +cd .. +# This hideous code is because we can't use python package option in grpc. +# See https://github.com/protocolbuffers/protobuf/issues/7061 for an explanation. +# We need to import these packages as a module. +sed -i 's/from armada/from armada_client.armada/g' client/python/armada_client/armada/*.py +sed -i 's/from github.com/from armada_client.github.com/g' client/python/armada_client/armada/*.py +sed -i 's/from google.api/from armada_client.google.api/g' client/python/armada_client/armada/*.py +sed -i 's/from google.api/from armada_client.google.api/g' client/python/armada_client/google/api/*.py + +find client/python/armada_client/ -name '*.py' | xargs sed -i 's/from k8s.io/from armada_client.k8s.io/g' + +# Generate better docs for the client +export PYTHONPATH=${PWD}/client/python +python3 ${PWD}/client/python/armada_client/gen/event_typings.py + +find client/python/armada_client/k8s -name '*.pyi' | xargs sed -i 's/ k8s/ armada_client.k8s/g' +find client/python/armada_client/k8s -name '*.pyi' | xargs sed -i 's/\[k8s/\[armada_client.k8s/g' +find client/python/armada_client/k8s/io -name '*.pyi' | xargs sed -i 's/import k8s.io/import armada_client.k8s.io/g'