Merge branch 'airbytehq:main' into remove_long_standing_connections_on_sql_state_and_catalog_backends

Author: benjaminwilen

.github/workflows/auto_merge_notification.yml

@@ -14,7 +14,7 @@ jobs:
     steps:
       - name: Add Auto-Merge Notice
         if: github.event.action == 'auto_merge_enabled'
-        uses: bcgov/action-pr-description-add@main
+        uses: bcgov/action-pr-description-add@v2.0.0
         with:
           add_markdown: |
             > [!IMPORTANT]
@@ -24,6 +24,8 @@ jobs:
 
       - name: Remove Auto-Merge Notice
         if: github.event.action == 'auto_merge_disabled'
-        uses: bcgov/action-pr-description-add@main
+        uses: bcgov/action-pr-description-add@v2.0.0
         with:
-          add_markdown: ""  # Empty string to remove the notice
+          add_markdown: |
+            > [!NOTE]
+            > **Auto-merge may have been disabled. Please check the PR status to confirm.**

.github/workflows/python_lint.yml

@@ -43,7 +43,7 @@ jobs:
     - name: Set up Poetry
       uses: Gr1N/setup-poetry@v9
       with:
-        poetry-version: "1.7.1"
+        poetry-version: "2.0.1"
     - name: Set up Python
       uses: actions/setup-python@v5
       with:
@@ -66,7 +66,7 @@ jobs:
     - name: Set up Poetry
       uses: Gr1N/setup-poetry@v9
       with:
-        poetry-version: "1.7.1"
+        poetry-version: "2.0.1"
     - name: Set up Python
       uses: actions/setup-python@v5
       with:

.github/workflows/python_pytest.yml

@@ -48,16 +48,6 @@ jobs:
         --durations=5 --exitfirst
         -m "not slow and not requires_creds and not linting and not flaky"
 
-    - name: Run Pytest with Coverage (Flaky Tests Only)
-      timeout-minutes: 60
-      continue-on-error: true
-      env:
-        GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }}
-      run: >
-        poetry run coverage run -m pytest
-        --durations=5 --exitfirst
-        -m "flaky and not slow and not requires_creds"
-
     - name: Print Coverage Report
       if: always()
       run: poetry run coverage report
@@ -75,6 +65,13 @@ jobs:
         name: fasttest-coverage
         path: htmlcov/
 
+    - name: Upload logs to GitHub Artifacts
+      if: failure()
+      uses: actions/upload-artifact@v4
+      with:
+        name: pytest-fast-test-logs
+        path: /tmp/airbyte/logs/
+
   pytest-no-creds:
     name: Pytest (No Creds)
     runs-on: ubuntu-latest
@@ -85,7 +82,7 @@ jobs:
     - name: Set up Poetry
       uses: Gr1N/setup-poetry@v9
       with:
-        poetry-version: "1.7.1"
+        poetry-version: "2.0.1"
     - name: Set up Python
       uses: actions/setup-python@v5
       with:
@@ -122,6 +119,13 @@ jobs:
         name: nocreds-test-coverage
         path: htmlcov/
 
+    - name: Upload logs to GitHub Artifacts
+      if: failure()
+      uses: actions/upload-artifact@v4
+      with:
+        name: pytest-no-creds-test-logs
+        path: /tmp/airbyte/logs/
+
   pytest:
     name: Pytest (All, Python ${{ matrix.python-version }}, ${{ matrix.os }})
     # Don't run on forks. Run on pushes to main, and on PRs that are not from forks.
@@ -133,7 +137,7 @@ jobs:
         python-version: [
           '3.10',
           '3.11',
-          '3.12',
+          # '3.12', # Commented out: Certain tests don't work in Python 3.12, although PyAirbyte itself does work on 3.12
         ]
         os: [
           Ubuntu,
@@ -153,7 +157,7 @@ jobs:
     - name: Set up Poetry
       uses: Gr1N/setup-poetry@v9
       with:
-        poetry-version: "1.7.1"
+        poetry-version: "2.0.1"
     - name: Set up Python
       uses: actions/setup-python@v5
       with:
@@ -189,6 +193,13 @@ jobs:
         name: py${{ matrix.python-version }}-${{ matrix.os }}-test-coverage
         path: htmlcov/
 
+    - name: Upload logs to GitHub Artifacts
+      if: failure()
+      uses: actions/upload-artifact@v4
+      with:
+        name: py${{ matrix.python-version }}-${{ matrix.os }}-test-logs
+        path: /tmp/airbyte/logs/
+
   dependency-analysis:
     name: Dependency Analysis with Deptry
     runs-on: ubuntu-latest
@@ -202,7 +213,7 @@ jobs:
     - name: Set up Poetry
       uses: Gr1N/setup-poetry@v9
       with:
-        poetry-version: "1.7.1"
+        poetry-version: "2.0.1"
     - name: Install dependencies
       run: poetry install
 

.github/workflows/semantic_pr_check.yml

@@ -47,3 +47,13 @@ jobs:
           # scopes: |
           #   core
           #   ui
+
+      - name: Check for "do not merge" in PR title
+        if: ${{ github.event.pull_request.draft == false }}
+        uses: actions/github-script@v6
+        with:
+          script: |
+            const title = context.payload.pull_request.title.toLowerCase();
+            if (title.includes('do not merge') || title.includes('do-not-merge')) {
+              core.setFailed('PR title contains "do not merge" or "do-not-merge". Please remove this before merging.');
+            }

airbyte/_connector_base.py

@@ -6,6 +6,7 @@
 import abc
 import json
 from pathlib import Path
+from time import sleep
 from typing import TYPE_CHECKING, Any, Literal
 
 import jsonschema
@@ -324,6 +325,8 @@ def check(self) -> None:
                             name=self.name,
                             state=EventState.FAILED,
                         )
+                        # Give logs a chance to be flushed.
+                        sleep(1)
                         raise exc.AirbyteConnectorCheckFailedError(
                             connector_name=self.name,
                             help_url=self.docs_url,

airbyte/_executors/base.py

@@ -185,6 +185,14 @@ def _cli(self) -> list[str]:
         """
         ...
 
+    def map_cli_args(self, args: list[str]) -> list[str]:
+        """Map CLI args if needed.
+
+        By default, this is a no-op. Subclasses may override this method in order to
+        map CLI args into the format expected by the connector.
+        """
+        return args
+
     def execute(
         self,
         args: list[str],
@@ -195,8 +203,9 @@ def execute(
 
         If stdin is provided, it will be passed to the subprocess as STDIN.
         """
+        mapped_args = self.map_cli_args(args)
         with _stream_from_subprocess(
-            [*self._cli, *args],
+            [*self._cli, *mapped_args],
             stdin=stdin,
         ) as stream_lines:
             yield from stream_lines

airbyte/_executors/declarative.py

@@ -18,6 +18,7 @@
 
 
 if TYPE_CHECKING:
+    from argparse import Namespace
     from collections.abc import Iterator
 
     from airbyte._message_iterators import AirbyteMessageIterator
@@ -102,7 +103,9 @@ def execute(
         """Execute the declarative source."""
         _ = stdin  # Not used
         source_entrypoint = AirbyteEntrypoint(self.declarative_source)
-        parsed_args = source_entrypoint.parse_args(args)
+
+        mapped_args: list[str] = self.map_cli_args(args)
+        parsed_args: Namespace = source_entrypoint.parse_args(mapped_args)
         yield from source_entrypoint.run(parsed_args)
 
     def ensure_installation(self, *, auto_fix: bool = True) -> None:

airbyte/_executors/docker.py

@@ -1,22 +1,33 @@
 # Copyright (c) 2023 Airbyte, Inc., all rights reserved.
 from __future__ import annotations
 
+import logging
 import shutil
+from pathlib import Path
 from typing import NoReturn
 
 from airbyte import exceptions as exc
 from airbyte._executors.base import Executor
 
 
+logger = logging.getLogger("airbyte")
+
+
+DEFAULT_AIRBYTE_CONTAINER_TEMP_DIR = "/airbyte/tmp"
+"""Default temp dir in an Airbyte connector's Docker image."""
+
+
 class DockerExecutor(Executor):
     def __init__(
         self,
         name: str | None = None,
         *,
         executable: list[str],
         target_version: str | None = None,
+        volumes: dict[Path, str] | None = None,
     ) -> None:
         self.executable: list[str] = executable
+        self.volumes: dict[Path, str] = volumes or {}
         name = name or executable[0]
         super().__init__(name=name, target_version=target_version)
 
@@ -56,3 +67,40 @@ def uninstall(self) -> NoReturn:
     def _cli(self) -> list[str]:
         """Get the base args of the CLI executable."""
         return self.executable
+
+    def map_cli_args(self, args: list[str]) -> list[str]:
+        """Map local file paths to the container's volume paths."""
+        new_args = []
+        for arg in args:
+            if Path(arg).exists():
+                # This is a file path and we need to map it to the same file within the
+                # relative path of the file within the container's volume.
+                for local_volume, container_path in self.volumes.items():
+                    if Path(arg).is_relative_to(local_volume):
+                        logger.debug(
+                            f"Found file input path `{arg}` "
+                            f"relative to container-mapped volume: {local_volume}"
+                        )
+                        mapped_path = Path(container_path) / Path(arg).relative_to(local_volume)
+                        logger.debug(f"Mapping `{arg}` -> `{mapped_path}`")
+                        new_args.append(str(mapped_path))
+                        break
+                else:
+                    # No break reached; a volume was found for this file path
+                    logger.warning(
+                        f"File path `{arg}` is not relative to any volume path "
+                        f"in the provided volume mappings: {self.volumes}. "
+                        "The file may not be available to the container at runtime."
+                    )
+                    new_args.append(arg)
+
+            else:
+                new_args.append(arg)
+
+        if args != new_args:
+            logger.debug(
+                f"Mapping local-to-container CLI args: {args} -> {new_args} "
+                f"based upon volume definitions: {self.volumes}"
+            )
+
+        return new_args

airbyte/_executors/util.py

@@ -11,7 +11,7 @@
 
 from airbyte import exceptions as exc
 from airbyte._executors.declarative import DeclarativeExecutor
-from airbyte._executors.docker import DockerExecutor
+from airbyte._executors.docker import DEFAULT_AIRBYTE_CONTAINER_TEMP_DIR, DockerExecutor
 from airbyte._executors.local import PathExecutor
 from airbyte._executors.python import VenvExecutor
 from airbyte._util.meta import which
@@ -42,8 +42,8 @@ def _try_get_source_manifest(
 
     Raises:
         - `PyAirbyteInputError`: If `source_name` is `None`.
-        - `HTTPError`: If fetching the URL was unsuccessful.
-        - `YAMLError`: If parsing the YAML failed.
+        - `AirbyteConnectorInstallationError`: If the registry file cannot be downloaded or if the
+          manifest YAML cannot be parsed.
     """
     if source_name is None:
         raise exc.PyAirbyteInputError(
@@ -62,7 +62,16 @@ def _try_get_source_manifest(
         url=manifest_url,
         headers={"User-Agent": f"PyAirbyte/{get_version()}"},
     )
-    response.raise_for_status()  # Raise HTTPError exception if the download failed
+    try:
+        response.raise_for_status()  # Raise HTTPError exception if the download failed
+    except requests.exceptions.HTTPError as ex:
+        raise exc.AirbyteConnectorInstallationError(
+            message="Failed to download the connector manifest.",
+            context={
+                "manifest_url": manifest_url,
+            },
+        ) from ex
+
     try:
         return cast("dict", yaml.safe_load(response.text))
     except yaml.YAMLError as ex:
@@ -115,7 +124,7 @@ def _get_local_executor(
     )
 
 
-def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915 # Too many branches/arugments/statements
+def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915, C901 # Too many branches/arguments/statements
     name: str,
     *,
     version: str | None = None,
@@ -139,6 +148,19 @@ def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915 # Too many branch
             bool(source_manifest),
         ]
     )
+
+    if version and pip_url:
+        raise exc.PyAirbyteInputError(
+            message=(
+                "Cannot specify both version and pip_url. "
+                "Make sure to specify the connector version directly in the pip_url."
+            ),
+            context={
+                "version": version,
+                "pip_url": pip_url,
+            },
+        )
+
     if install_method_count > 1:
         raise exc.PyAirbyteInputError(
             message=(
@@ -189,6 +211,7 @@ def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915 # Too many branch
                     source_manifest = True
                 case InstallType.PYTHON:
                     pip_url = metadata.pypi_package_name
+                    pip_url = f"{pip_url}=={version}" if version else pip_url
                 case _:
                     docker_image = True
 
@@ -217,21 +240,24 @@ def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915 # Too many branch
         if ":" not in docker_image:
             docker_image = f"{docker_image}:{version or 'latest'}"
 
-        temp_dir = TEMP_DIR_OVERRIDE or Path(tempfile.gettempdir())
+        host_temp_dir = TEMP_DIR_OVERRIDE or Path(tempfile.gettempdir())
+        container_temp_dir = DEFAULT_AIRBYTE_CONTAINER_TEMP_DIR
 
         local_mount_dir = Path().absolute() / name
         local_mount_dir.mkdir(exist_ok=True)
 
+        volumes = {
+            local_mount_dir: "/local",
+            host_temp_dir: container_temp_dir,
+        }
         docker_cmd = [
             "docker",
             "run",
             "--rm",
             "-i",
-            "--volume",
-            f"{local_mount_dir}:/local/",
-            "--volume",
-            f"{temp_dir}:{temp_dir}",
         ]
+        for local_dir, container_dir in volumes.items():
+            docker_cmd.extend(["--volume", f"{local_dir}:{container_dir}"])
 
         if use_host_network is True:
             docker_cmd.extend(["--network", "host"])
@@ -241,6 +267,7 @@ def get_connector_executor(  # noqa: PLR0912, PLR0913, PLR0915 # Too many branch
         return DockerExecutor(
             name=name,
             executable=docker_cmd,
+            volumes=volumes,
         )
 
     if source_manifest: