diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6647e689..4fa36d05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,10 @@ on: pull_request: branches: [ main ] +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + env: PIPELINE_FAMILY: "general" @@ -21,6 +25,7 @@ jobs: with: enable-cache: true cache-dependency-glob: "uv.lock" + cache-suffix: lint - name: Set up Python ${{ env.PYTHON_VERSION }} run: uv python install ${{ env.PYTHON_VERSION }} - name: Install lint dependencies @@ -47,19 +52,24 @@ jobs: with: enable-cache: true cache-dependency-glob: "uv.lock" + cache-suffix: test - name: Set up Python ${{ env.PYTHON_VERSION }} run: uv python install ${{ env.PYTHON_VERSION }} - - name: Install dependencies and run core tests + - name: Install system dependencies run: | sudo apt-get update && sudo apt-get install --yes poppler-utils libreoffice - uv sync --group test --locked make install-pandoc - make install-nlp-models sudo add-apt-repository -y ppa:alex-p/tesseract-ocr5 sudo apt-get install -y tesseract-ocr tesseract-ocr-kor tesseract --version - make test - make check-coverage + - name: Install test dependencies + run: uv sync --group test --locked + - name: Install NLP models + run: make install-nlp-models + - name: Run core tests + run: make test + - name: Check coverage + run: make check-coverage changelog: runs-on: opensource-linux-8core @@ -77,7 +87,6 @@ jobs: - if: steps.changes.outputs.src == 'true' && github.ref != 'refs/heads/main' uses: dangoslen/changelog-enforcer@v3 - # TODO - figure out best practice for caching docker images test_dockerfile: runs-on: opensource-linux-8core needs: lint @@ -90,6 +99,7 @@ jobs: with: enable-cache: true cache-dependency-glob: "uv.lock" + cache-suffix: smoke - name: Set up Python ${{ env.PYTHON_VERSION }} run: uv python install ${{ env.PYTHON_VERSION }} - name: Free up disk space @@ -112,8 +122,9 @@ jobs: echo "Disk usage after cleanup:" df -h + - name: Install smoke test dependencies + run: uv sync --only-group smoke --locked + - name: Build Dockerfile + run: make docker-build - name: Test Dockerfile - run: | - uv sync --group test --locked - make docker-build - make docker-test + run: make docker-test diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index f2cf506b..07beba43 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -80,10 +80,11 @@ jobs: with: enable-cache: true cache-dependency-glob: "uv.lock" + cache-suffix: smoke - name: Set up Python ${{ env.PYTHON_VERSION }} run: uv python install ${{ env.PYTHON_VERSION }} - name: Install test dependencies - run: uv sync --group test --locked + run: uv sync --only-group smoke --locked - name: Test image run: | export DOCKER_IMAGE="$DOCKER_BUILD_REPOSITORY:${{ matrix.arch }}-$SHORT_SHA" @@ -100,27 +101,22 @@ jobs: steps: - name: Checkout code uses: actions/checkout@v5 - - name: Set SHORT_SHA - run: echo "SHORT_SHA=$(git rev-parse --short HEAD)" >> $GITHUB_ENV + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v4 - name: Login to Quay.io uses: docker/login-action@v4 with: registry: quay.io username: ${{ secrets.QUAY_IO_ROBOT_USERNAME }} password: ${{ secrets.QUAY_IO_ROBOT_TOKEN }} - - name: Pull AMD image - run: | - docker pull $DOCKER_BUILD_REPOSITORY:amd64-$SHORT_SHA - - name: Pull ARM image - run: | - docker pull $DOCKER_BUILD_REPOSITORY:arm64-$SHORT_SHA - - name: Push AMD and ARM tags + - name: Update architecture cache tags run: | - # these are used to construct the final manifest but also cache-from in subsequent runs - docker tag $DOCKER_BUILD_REPOSITORY:amd64-$SHORT_SHA $DOCKER_BUILD_REPOSITORY:amd64 - docker push $DOCKER_BUILD_REPOSITORY:amd64 - docker tag $DOCKER_BUILD_REPOSITORY:arm64-$SHORT_SHA $DOCKER_BUILD_REPOSITORY:arm64 - docker push $DOCKER_BUILD_REPOSITORY:arm64 + docker buildx imagetools create --prefer-index=false \ + -t $DOCKER_BUILD_REPOSITORY:amd64 \ + $DOCKER_BUILD_REPOSITORY:amd64-$SHORT_SHA + docker buildx imagetools create --prefer-index=false \ + -t $DOCKER_BUILD_REPOSITORY:arm64 \ + $DOCKER_BUILD_REPOSITORY:arm64-$SHORT_SHA - name: Push multiarch manifest run: | VERSION=$(grep -oP '(?<=__version__ = ")[^"]+' prepline_general/api/__version__.py) @@ -128,5 +124,5 @@ jobs: -t ${DOCKER_REPOSITORY}:latest \ -t ${DOCKER_REPOSITORY}:$SHORT_SHA \ -t ${DOCKER_REPOSITORY}:$VERSION \ - $DOCKER_BUILD_REPOSITORY:amd64 \ - $DOCKER_BUILD_REPOSITORY:arm64 + $DOCKER_BUILD_REPOSITORY:amd64-$SHORT_SHA \ + $DOCKER_BUILD_REPOSITORY:arm64-$SHORT_SHA diff --git a/pyproject.toml b/pyproject.toml index e2127676..70220775 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,11 @@ dependencies = [ ] [dependency-groups] +smoke = [ + "pytest >=9.0.2, <10.0.0", + "pandas >=3.0.0, <4.0.0", + "requests >=2.32.0, <3.0.0", +] lint = [ "ruff >=0.15.0, <1.0.0", "mypy >=1.19.1, <2.0.0", diff --git a/scripts/docker-smoke-test.sh b/scripts/docker-smoke-test.sh index 1df9ec6b..534535b2 100755 --- a/scripts/docker-smoke-test.sh +++ b/scripts/docker-smoke-test.sh @@ -80,7 +80,7 @@ await_server_ready 8000 # Smoke Tests ####################### echo Running smoke tests with SKIP_INFERENCE_TESTS: "$SKIP_INFERENCE_TESTS" -PYTHONPATH=. SKIP_INFERENCE_TESTS=$SKIP_INFERENCE_TESTS uv run pytest -vv scripts/smoketest.py +PYTHONPATH=. SKIP_INFERENCE_TESTS=$SKIP_INFERENCE_TESTS uv run --no-sync pytest --durations=20 -vv scripts/smoketest.py ####################### # Test parallel vs single mode diff --git a/scripts/parallel-mode-test.sh b/scripts/parallel-mode-test.sh index ff5c7ec9..1a4a7c12 100755 --- a/scripts/parallel-mode-test.sh +++ b/scripts/parallel-mode-test.sh @@ -1,73 +1,40 @@ #!/usr/bin/env bash -# parallel-mode-test.sh -# Iterate a list of curl commands, and run each one against two instances of the api -# The smoke test will start one container with parallel mode and one without, and -# diff the two outputs to make sure parallel mode does not alter the response. -# Note the filepaths assume you ran this from the top level - -# shellcheck disable=SC2317,SC2086 # SC2317: trap functions appear unreachable, SC2086: curl params require word splitting +set -euo pipefail base_url_1=$1 base_url_2=$2 -MAX_RETRIES=3 +results_dir=$(mktemp -d) +trap 'rm -rf "$results_dir"' EXIT -curl_with_retry() { - local curl_command=$1 +request_elements() { + local base_url=$1 local output_file=$2 + shift 2 - for attempt in $(seq 1 $MAX_RETRIES); do - $curl_command 2> /dev/null | jq -S 'del(..|.parent_id?)' > "$output_file" - if [ -s "$output_file" ]; then - return 0 - fi - echo " Attempt $attempt/$MAX_RETRIES failed, retrying in 5s..." - sleep 5 - done - - echo " All $MAX_RETRIES attempts failed!" - $curl_command - return 1 + curl --fail --silent --show-error --retry 2 --max-time 300 \ + "$base_url/general/v0/general" \ + -F files=@sample-docs/layout-parser-paper.pdf \ + "$@" > "$output_file" + jq -e 'type == "array" and length > 0' "$output_file" > /dev/null } -declare -a curl_params=( - "-F files=@sample-docs/layout-parser-paper.pdf -F 'strategy=fast'" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'strategy=auto" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'strategy=hi_res'" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'coordinates=true'" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'encoding=utf-8'" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'include_page_breaks=true'" - "-F files=@sample-docs/layout-parser-paper.pdf -F 'hi_res_model_name=yolox'" -) - -for params in "${curl_params[@]}" -do - curl_command="curl $base_url_1/general/v0/general $params" - echo Testing: "$curl_command" - - # Run in single mode - # Note(austin): Parallel mode screws up hierarchy! While we deal with that, - # let's ignore parent_id fields in the results - if ! curl_with_retry "$curl_command" output.json; then - echo Command failed! - exit 1 - fi - original_length=$(jq 'length' output.json) - - # Run in parallel mode - curl_command="curl $base_url_2/general/v0/general $params" - if ! curl_with_retry "$curl_command" parallel_output.json; then - echo Command failed! - exit 1 - fi - parallel_length=$(jq 'length' parallel_output.json) - - if ! [[ "$original_length" == "$parallel_length" ]]; then - echo Parallel mode returned a different number of elements! - echo Params: "$params" - exit 1 - fi - - rm -f output.json parallel_output.json - echo +for option in strategy=fast strategy=auto strategy=hi_res coordinates=true encoding=utf-8 include_page_breaks=true hi_res_model_name=yolox; do + # Exercise forwarding options without repeating inference for each one. + case "$option" in + strategy=*) form_args=(-F "$option") ;; + hi_res_model_name=*) form_args=(-F strategy=hi_res -F "$option") ;; + *) form_args=(-F strategy=fast -F "$option") ;; + esac + + echo "Testing: $option" + request_elements "$base_url_1" "$results_dir/single.json" "${form_args[@]}" + request_elements "$base_url_2" "$results_dir/parallel.json" "${form_args[@]}" + + original_length=$(jq 'length' "$results_dir/single.json") + parallel_length=$(jq 'length' "$results_dir/parallel.json") + if [[ "$original_length" != "$parallel_length" ]]; then + echo "Parallel mode returned a different number of elements for $option" + exit 1 + fi done diff --git a/scripts/smoketest.py b/scripts/smoketest.py index 6c7e7683..b89a6269 100644 --- a/scripts/smoketest.py +++ b/scripts/smoketest.py @@ -146,7 +146,6 @@ def test_happy_path_all_types(extension, example_filename: str, content_type: st assert len(df) == len(json_response.json()) -@pytest.mark.parametrize("output_format", ["application/json", "text/csv"]) @pytest.mark.parametrize( "filenames_to_gzip, filenames_verbatim, uncompressed_content_type", [ @@ -173,7 +172,6 @@ def test_happy_path_all_types(extension, example_filename: str, content_type: st ], ) def test_gzip_sending( - output_format: str, filenames_to_gzip: List[str], filenames_verbatim: List[str], uncompressed_content_type: str, diff --git a/test_general/test_parallel_mode_script.py b/test_general/test_parallel_mode_script.py new file mode 100644 index 00000000..628b5e45 --- /dev/null +++ b/test_general/test_parallel_mode_script.py @@ -0,0 +1,88 @@ +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "parallel-mode-test.sh" + + +@pytest.fixture +def run_comparisons(tmp_path): + curl = tmp_path / "curl" + curl.write_text( + f"#!{sys.executable}\n" + "import json, os, sys\n" + "with open(os.environ['REQUEST_LOG'], 'a') as log:\n" + " log.write(json.dumps(sys.argv[1:]) + '\\n')\n" + "if os.environ.get('HTTP_FAILURE'):\n" + " sys.exit(22)\n" + "body = os.environ.get('RESPONSE_BODY', '[{\"text\": \"content\"}]')\n" + "if os.environ.get('MISMATCH') and any('parallel/' in arg for arg in sys.argv):\n" + " body = '[{}, {}]'\n" + "print(body)\n" + ) + curl.chmod(0o755) + log = tmp_path / "requests.jsonl" + + def run(**overrides): + result = subprocess.run( + ["bash", str(SCRIPT), "http://single", "http://parallel"], + env={ + **os.environ, + "PATH": f"{tmp_path}:{os.environ['PATH']}", + "REQUEST_LOG": str(log), + **overrides, + }, + capture_output=True, + text=True, + check=False, + ) + requests = [json.loads(line) for line in log.read_text().splitlines()] + return result, requests + + return run + + +def test_comparisons_send_valid_options_to_both_servers(run_comparisons): + result, requests = run_comparisons() + assert result.returncode == 0, result.stderr + assert len(requests) == 14 + expected = [ + {"strategy": "fast"}, + {"strategy": "auto"}, + {"strategy": "hi_res"}, + {"strategy": "fast", "coordinates": "true"}, + {"strategy": "fast", "encoding": "utf-8"}, + {"strategy": "fast", "include_page_breaks": "true"}, + {"strategy": "hi_res", "hi_res_model_name": "yolox"}, + ] + for index, args in enumerate(requests): + fields = dict(args[i + 1].split("=", 1) for i, arg in enumerate(args) if arg == "-F") + assert fields.pop("files") == "@sample-docs/layout-parser-paper.pdf" + assert fields == expected[index // 2] + server = "single" if index % 2 == 0 else "parallel" + assert f"http://{server}/general/v0/general" in args + assert "--fail" in args + + +@pytest.mark.parametrize("body", ['{"detail": "server error"}', "[]", "invalid JSON"]) +def test_comparisons_reject_invalid_responses(run_comparisons, body): + result, requests = run_comparisons(RESPONSE_BODY=body) + assert result.returncode != 0 + assert len(requests) == 1 + + +def test_comparisons_reject_http_failure(run_comparisons): + result, requests = run_comparisons(HTTP_FAILURE="1") + assert result.returncode != 0 + assert len(requests) == 1 + + +def test_comparisons_reject_different_element_counts(run_comparisons): + result, requests = run_comparisons(MISMATCH="1") + assert result.returncode != 0 + assert len(requests) == 2 + assert "different number of elements" in result.stdout diff --git a/uv.lock b/uv.lock index 904ed082..2f97851c 100644 --- a/uv.lock +++ b/uv.lock @@ -2215,6 +2215,11 @@ lint = [ { name = "ruff" }, { name = "types-requests" }, ] +smoke = [ + { name = "pandas" }, + { name = "pytest" }, + { name = "requests" }, +] test = [ { name = "deepdiff" }, { name = "httpx" }, @@ -2246,6 +2251,11 @@ lint = [ { name = "ruff", specifier = ">=0.15.0,<1.0.0" }, { name = "types-requests", specifier = ">=2.32.4,<3.0.0" }, ] +smoke = [ + { name = "pandas", specifier = ">=3.0.0,<4.0.0" }, + { name = "pytest", specifier = ">=9.0.2,<10.0.0" }, + { name = "requests", specifier = ">=2.32.0,<3.0.0" }, +] test = [ { name = "deepdiff", specifier = ">=8.6.1,<9.0.0" }, { name = "httpx", specifier = ">=0.28.1,<1.0.0" },