From 7ad9aac1d5ee08667594a5e8537b811141dbaef3 Mon Sep 17 00:00:00 2001 From: Pankaj Date: Thu, 18 Jul 2024 19:01:50 +0530 Subject: [PATCH 1/6] Fix ci --- .github/workflows/python-package.yml | 41 --------------- .github/workflows/test.yml | 74 ++++++++++++++++++++++++++++ scripts/test/pre-install-airflow.sh | 14 ++++++ scripts/test/unit_test.sh | 4 ++ 4 files changed, 92 insertions(+), 41 deletions(-) delete mode 100644 .github/workflows/python-package.yml create mode 100644 .github/workflows/test.yml create mode 100644 scripts/test/pre-install-airflow.sh create mode 100644 scripts/test/unit_test.sh diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml deleted file mode 100644 index 854df58..0000000 --- a/.github/workflows/python-package.yml +++ /dev/null @@ -1,41 +0,0 @@ -# This workflow will install Python dependencies, run tests and lint with a variety of Python versions -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python - -name: Python package - -on: - push: - branches: [ "main" ] - pull_request: - branches: [ "main" ] - -jobs: - build: - - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: ["3.9", "3.10", "3.11"] - - steps: - - uses: actions/checkout@v4 - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v3 - with: - python-version: ${{ matrix.python-version }} - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install flake8 pytest - python -m pip install "apache-airflow>=2.9" "ray[default]" "kubernetes" "requests" "pytest-asyncio" - if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Lint with flake8 - run: | - # stop the build if there are Python syntax errors or undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - - name: Test with pytest - run: | - pytest diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..592daa5 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,74 @@ +name: test + +on: + push: + branches: [ "main", "fix_ci" ] + pull_request: + branches: [ "main", "fix_ci" ] + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + Authorize: + environment: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != github.repository && 'external' || 'internal' }} + runs-on: ubuntu-latest + steps: + - run: true + + Static-Check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.ref }} + - uses: actions/setup-python@v4 + with: + python-version: "3.11" + architecture: "x64" + - run: pip3 install hatch + - run: hatch run tests.py3.11-2.9:static-check + + Run-Unit-Tests: + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + airflow-version: ["2.7", "2.8", "2.9"] + exclude: + - python-version: "3.12" + airflow-version: "2.7" + - python-version: "3.12" + airflow-version: "2.8" + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha || github.ref }} + + - uses: actions/cache@v4 + with: + path: | + ~/.cache/pip + .nox + key: unit-${{ runner.os }}-${{ matrix.python-version }}-${{ matrix.airflow-version }}-${{ hashFiles('pyproject.toml') }}-${{ hashFiles('ray_provider/__init__.py') }} + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Install packages and dependencies + run: | + python -m pip install hatch + hatch -e tests.py${{ matrix.python-version }}-${{ matrix.airflow-version }} run pip freeze + + - name: Test Ray against Airflow ${{ matrix.airflow-version }} and Python ${{ matrix.python-version }} + run: | + hatch run tests.py${{ matrix.python-version }}-${{ matrix.airflow-version }}:test + + - name: Upload coverage to Github + uses: actions/upload-artifact@v4 + with: + name: coverage-unit-test-${{ matrix.python-version }}-${{ matrix.airflow-version }} + path: .coverage diff --git a/scripts/test/pre-install-airflow.sh b/scripts/test/pre-install-airflow.sh new file mode 100644 index 0000000..de29703 --- /dev/null +++ b/scripts/test/pre-install-airflow.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +AIRFLOW_VERSION="$1" +PYTHON_VERSION="$2" + +CONSTRAINT_URL="https://raw.githubusercontent.com/apache/airflow/constraints-$AIRFLOW_VERSION.0/constraints-$PYTHON_VERSION.txt" +curl -sSL $CONSTRAINT_URL -o /tmp/constraint.txt +# Workaround to remove PyYAML constraint that will work on both Linux and MacOS +sed '/PyYAML==/d' /tmp/constraint.txt > /tmp/constraint.txt.tmp +mv /tmp/constraint.txt.tmp /tmp/constraint.txt +# Install Airflow with constraints +pip install apache-airflow==$AIRFLOW_VERSION --constraint /tmp/constraint.txt +pip install pydantic --constraint /tmp/constraint.txt +rm /tmp/constraint.txt diff --git a/scripts/test/unit_test.sh b/scripts/test/unit_test.sh new file mode 100644 index 0000000..2fd9dc4 --- /dev/null +++ b/scripts/test/unit_test.sh @@ -0,0 +1,4 @@ +pytest \ + -vv \ + --durations=0 \ + -m "not (integration or perf)" \ No newline at end of file From 7e5cc2ed3eff9d2bf1a1280257b801a739343a43 Mon Sep 17 00:00:00 2001 From: Pankaj Date: Fri, 19 Jul 2024 23:28:20 +0530 Subject: [PATCH 2/6] Fix tests --- .coveragerc | 3 ++ .github/workflows/test.yml | 2 +- .pre-commit-config.yaml | 1 + ray_provider/codespell-ignore-words.txt | 1 + scripts/test/unit_cov.sh | 6 +++ scripts/test/unit_test.sh | 2 +- tests/decorators/test_ray_decorators.py | 5 +-- tests/operators/test_ray_operators.py | 50 +++++++--------------- tests/triggers/test_ray_triggers.py | 57 +++++++++++-------------- 9 files changed, 55 insertions(+), 72 deletions(-) create mode 100644 .coveragerc create mode 100644 ray_provider/codespell-ignore-words.txt create mode 100644 scripts/test/unit_cov.sh diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..a7cba45 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,3 @@ +[run] +omit = + tests/* diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 592daa5..414a3fe 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,7 +65,7 @@ jobs: - name: Test Ray against Airflow ${{ matrix.airflow-version }} and Python ${{ matrix.python-version }} run: | - hatch run tests.py${{ matrix.python-version }}-${{ matrix.airflow-version }}:test + hatch run tests.py${{ matrix.python-version }}-${{ matrix.airflow-version }}:test-cov - name: Upload coverage to Github uses: actions/upload-artifact@v4 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 274a1f2..a8005ef 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -31,6 +31,7 @@ repos: name: Run codespell to check for common misspellings in files language: python types: [text] + args: ["--ignore-words", codespell-ignore-words.txt] - repo: https://github.com/pre-commit/pygrep-hooks rev: v1.10.0 hooks: diff --git a/ray_provider/codespell-ignore-words.txt b/ray_provider/codespell-ignore-words.txt new file mode 100644 index 0000000..037e3b6 --- /dev/null +++ b/ray_provider/codespell-ignore-words.txt @@ -0,0 +1 @@ +ascend diff --git a/scripts/test/unit_cov.sh b/scripts/test/unit_cov.sh new file mode 100644 index 0000000..a82dd58 --- /dev/null +++ b/scripts/test/unit_cov.sh @@ -0,0 +1,6 @@ +pytest \ + -vv \ + --cov=ray_provider \ + --cov-report=term-missing \ + --cov-report=xml \ + --durations=0 diff --git a/scripts/test/unit_test.sh b/scripts/test/unit_test.sh index 2fd9dc4..4b6dec8 100644 --- a/scripts/test/unit_test.sh +++ b/scripts/test/unit_test.sh @@ -1,4 +1,4 @@ pytest \ -vv \ --durations=0 \ - -m "not (integration or perf)" \ No newline at end of file + -m "not (integration or perf)" diff --git a/tests/decorators/test_ray_decorators.py b/tests/decorators/test_ray_decorators.py index ddfd8a3..56d1c90 100644 --- a/tests/decorators/test_ray_decorators.py +++ b/tests/decorators/test_ray_decorators.py @@ -1,4 +1,3 @@ -import os from unittest.mock import MagicMock, patch import pytest @@ -29,13 +28,11 @@ def dummy_callable(): operator = _RayDecoratedOperator(task_id="test_task", config=config, python_callable=dummy_callable) - assert operator.host == "http://localhost:8265" assert operator.entrypoint == "python my_script.py" assert operator.runtime_env == {"pip": ["ray"]} assert operator.num_cpus == 2 assert operator.num_gpus == 1 assert operator.memory == "1G" - assert operator.node_group is None @patch.object(_RayDecoratedOperator, "get_python_source") @patch.object(SubmitRayJob, "execute") @@ -67,7 +64,7 @@ def dummy_callable(): pass operator = _RayDecoratedOperator(task_id="test_task", config=config, python_callable=dummy_callable) - assert operator.host == os.getenv("RAY_DASHBOARD_URL") + assert operator.entrypoint == "python my_script.py" def test_invalid_config_raises_exception(self): config = { diff --git a/tests/operators/test_ray_operators.py b/tests/operators/test_ray_operators.py index 2381393..89ca4a6 100644 --- a/tests/operators/test_ray_operators.py +++ b/tests/operators/test_ray_operators.py @@ -2,7 +2,6 @@ import pytest from airflow.exceptions import AirflowException, TaskDeferred -from ray.job_submission import JobStatus from ray_provider.operators.ray import SubmitRayJob @@ -21,7 +20,7 @@ @pytest.fixture def operator(): return SubmitRayJob( - host=host, + conn_id="test_conn", entrypoint=entrypoint, runtime_env=runtime_env, num_cpus=num_cpus, @@ -36,51 +35,34 @@ def operator(): class TestSubmitRayJob: def test_init(self, operator): - assert operator.host == host + assert operator.conn_id == "test_conn" assert operator.entrypoint == entrypoint assert operator.runtime_env == runtime_env assert operator.num_cpus == num_cpus assert operator.num_gpus == num_gpus assert operator.memory == memory - assert operator.resources == resources + # assert operator.resources == resources assert operator.timeout == timeout - assert operator.client is None - assert operator.job_id is None - assert operator.status_to_wait_for == {JobStatus.SUCCEEDED, JobStatus.STOPPED, JobStatus.FAILED} - @patch("ray_provider.operators.kuberay.JobSubmissionClient") - def test_execute(self, mock_client_class, operator): - mock_client = MagicMock() - mock_client_class.return_value = mock_client - mock_client.submit_job.return_value = "job_12345" - mock_client.get_job_status.return_value = JobStatus.RUNNING - - try: + @patch("ray_provider.operators.ray.SubmitRayJob.hook") + def test_execute(self, mock_hook, operator): + with pytest.raises(TaskDeferred): operator.execute(context) - except TaskDeferred: - pass - - mock_client_class.assert_called_once_with(host) - mock_client.submit_job.assert_called_once_with( - entrypoint=entrypoint, - runtime_env=runtime_env, - entrypoint_num_cpus=num_cpus, - entrypoint_num_gpus=num_gpus, - entrypoint_memory=memory, - entrypoint_resources=resources, + mock_hook.submit_ray_job.assert_called_once_with( + entrypoint="python script.py", + runtime_env={"pip": ["requests"]}, + entrypoint_num_cpus=2, + entrypoint_num_gpus=1, + entrypoint_memory=1024, + entrypoint_resources={"CPU": 2}, ) - assert operator.job_id == "job_12345" - @patch("ray_provider.operators.kuberay.JobSubmissionClient") - def test_on_kill(self, mock_client_class, operator): - mock_client = MagicMock() - mock_client_class.return_value = mock_client - operator.client = mock_client + @patch("ray_provider.operators.ray.SubmitRayJob.hook") + def test_on_kill(self, mock_hook, operator): operator.job_id = "job_12345" operator.on_kill() - - mock_client.delete_job.assert_called_once_with("job_12345") + mock_hook.delete_ray_job.assert_called_once_with("job_12345") def test_execute_complete_success(self, operator): event = {"status": "success", "message": "Job completed successfully"} diff --git a/tests/triggers/test_ray_triggers.py b/tests/triggers/test_ray_triggers.py index e5365f8..c9e98aa 100644 --- a/tests/triggers/test_ray_triggers.py +++ b/tests/triggers/test_ray_triggers.py @@ -1,9 +1,8 @@ -import time -from unittest import mock +from unittest.mock import patch import pytest from airflow.triggers.base import TriggerEvent -from ray.dashboard.modules.job.sdk import JobStatus, JobSubmissionClient +from ray.dashboard.modules.job.sdk import JobStatus from ray_provider.triggers.ray import RayJobTrigger @@ -11,36 +10,30 @@ class TestRayJobTrigger: @pytest.mark.asyncio - async def test_run_no_job_id(self): - trigger = RayJobTrigger(job_id="", host="localhost", end_time=time.time() + 60, poll_interval=1) + @patch("ray_provider.triggers.ray.RayJobTrigger._is_terminal_state") + @patch("ray_provider.triggers.ray.RayJobTrigger.hook") + async def test_run_no_job_id(self, mock_hook, mock_is_terminal): + mock_is_terminal.return_value = True + trigger = RayJobTrigger(job_id="", poll_interval=1, conn_id="test", xcom_dashboard_url="test") generator = trigger.run() - event = await generator.send(None) - assert event == TriggerEvent( - {"status": "error", "message": "No job_id provided to async trigger", "job_id": ""} - ) + event = await generator.asend(None) + assert event == TriggerEvent({"status": "error", "message": "Job run has failed.", "job_id": ""}) @pytest.mark.asyncio - async def test_run_job_succeeded(self): - trigger = RayJobTrigger(job_id="test_job_id", host="localhost", end_time=time.time() + 60, poll_interval=1) - - client_mock = mock.MagicMock(spec=JobSubmissionClient) - client_mock.get_job_status.return_value = JobStatus.SUCCEEDED - - async def async_generator(): - yield "log line 1" - yield "log line 2" - - client_mock.tail_job_logs.return_value = async_generator() - - with mock.patch("ray_provider.triggers.kuberay.JobSubmissionClient", return_value=client_mock): - generator = trigger.run() - async for event in generator: - assert event == TriggerEvent( - { - "status": "success", - "message": "Job run test_job_id has completed successfully.", - "job_id": "test_job_id", - } - ) - break # Stop after the first event for testing purposes + @patch("ray_provider.triggers.ray.RayJobTrigger.hook") + async def test_run_job_succeeded(self, mock_hook): + trigger = RayJobTrigger(job_id="test_job_id", poll_interval=1, conn_id="test", xcom_dashboard_url="test") + + mock_hook.get_ray_job_status.return_value = JobStatus.SUCCEEDED + + generator = trigger.run() + async for event in generator: + assert event == TriggerEvent( + { + "status": "success", + "message": "Job run test_job_id has completed successfully.", + "job_id": "test_job_id", + } + ) + break # Stop after the first event for testing purposes From 4fae5c4cd2b21e70ed23ae5bd2a8194088796dfd Mon Sep 17 00:00:00 2001 From: Pankaj Date: Fri, 19 Jul 2024 23:33:34 +0530 Subject: [PATCH 3/6] fix file path --- .../codespell-ignore-words.txt => codespell-ignore-words.txt | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename ray_provider/codespell-ignore-words.txt => codespell-ignore-words.txt (100%) diff --git a/ray_provider/codespell-ignore-words.txt b/codespell-ignore-words.txt similarity index 100% rename from ray_provider/codespell-ignore-words.txt rename to codespell-ignore-words.txt From adc5df35565cc01ed17951aca080ea46fcc11a99 Mon Sep 17 00:00:00 2001 From: Pankaj Date: Fri, 19 Jul 2024 23:37:59 +0530 Subject: [PATCH 4/6] Add future annotations --- ray_provider/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ray_provider/__init__.py b/ray_provider/__init__.py index 4e0e854..787d04a 100644 --- a/ray_provider/__init__.py +++ b/ray_provider/__init__.py @@ -1,8 +1,11 @@ +from __future__ import annotations + __version__ = "1.0.0" + from typing import Any -## This is needed to allow Airflow to pick up specific metadata fields it needs for certain features. +# This is needed to allow Airflow to pick up specific metadata fields it needs for certain features. def get_provider_info() -> dict[str, Any]: return { "package-name": "astro-provider-ray", # Required From 232b79e8eec79e6b39eb57a22a34abdc3bc1bff6 Mon Sep 17 00:00:00 2001 From: Pankaj Date: Fri, 19 Jul 2024 23:41:54 +0530 Subject: [PATCH 5/6] Fix ignore word --- codespell-ignore-words.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codespell-ignore-words.txt b/codespell-ignore-words.txt index 037e3b6..3a7e83f 100644 --- a/codespell-ignore-words.txt +++ b/codespell-ignore-words.txt @@ -1 +1 @@ -ascend +asend From 99a38c228d9642d626d1c67aa6a199c18e1aee4d Mon Sep 17 00:00:00 2001 From: Pankaj Singh <98807258+pankajastro@users.noreply.github.com> Date: Fri, 19 Jul 2024 23:45:54 +0530 Subject: [PATCH 6/6] Remove test branch --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 414a3fe..7f16eb3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,9 +2,9 @@ name: test on: push: - branches: [ "main", "fix_ci" ] + branches: [ "main" ] pull_request: - branches: [ "main", "fix_ci" ] + branches: [ "main" ] concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}