Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .github/workflows/dbt-integration-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: dbt-integration-tests

# Run dbt integration tests on PRs
on:
pull_request:
types:
- opened
- synchronize
- labeled
paths:
- 'sdk/python/feast/dbt/**'
- 'sdk/python/tests/integration/dbt/**'
- 'sdk/python/tests/unit/dbt/**'
- '.github/workflows/dbt-integration-tests.yml'

jobs:
dbt-integration-test:
if:
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
(github.event.action != 'labeled' && (contains(github.event.pull_request.labels.*.name, 'ok-to-test') || contains(github.event.pull_request.labels.*.name, 'approved') || contains(github.event.pull_request.labels.*.name, 'lgtm')))) &&
github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
runs-on: ubuntu-latest
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have a check for ok-to-test label so that resources don't get spin up for every PR raised, similar to other integration tests pattern
https://github.com/feast-dev/feast/blob/master/.github/workflows/pr_local_integration_tests.yml#L13

strategy:
matrix:
python-version: ["3.11", "3.12"]
env:
PYTHON: ${{ matrix.python-version }}
steps:
- uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
architecture: x64

- name: Install the latest version of uv
uses: astral-sh/setup-uv@v5
with:
enable-cache: true

- name: Install dependencies
run: make install-python-dependencies-ci

- name: Install dbt and dbt-duckdb
run: |
uv pip install --system dbt-core dbt-duckdb

- name: Run dbt integration tests
run: make test-python-integration-dbt

- name: Minimize uv cache
run: uv cache prune --ci
22 changes: 22 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,28 @@ test-python-integration-container: ## Run Python integration tests using Docker
python -m pytest -n 8 --integration sdk/python/tests \
) || echo "This script uses Docker, and it isn't running - please start the Docker Daemon and try again!";

test-python-integration-dbt: ## Run dbt integration tests
@echo "Running dbt integration tests..."
@cd sdk/python/tests/integration/dbt/test_dbt_project && \
echo "Installing dbt dependencies..." && \
dbt deps && \
echo "Building dbt models..." && \
dbt build
@cd sdk/python/tests/integration/dbt && \
echo "Setting up Feast project..." && \
mkdir -p feast_repo/data && \
echo "project: feast_dbt_test\nregistry: data/registry.db\nprovider: local\nonline_store:\n type: sqlite\n path: data/online_store.db" > feast_repo/feature_store.yaml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Makefile echo with \n escape sequences relies on shell-specific behavior, fails on bash

The echo command on line 200 uses \n to produce newlines in the generated feature_store.yaml, but POSIX echo behavior with escape sequences is implementation-defined. On systems where /bin/sh is bash (or when Make's SHELL is set to bash), echo does NOT interpret \n, producing a single-line file with literal \n text — resulting in invalid YAML that causes the subsequent feast dbt import command to fail.

Root Cause and Impact

The command:

echo "project: feast_dbt_test\nregistry: data/registry.db\n..." > feast_repo/feature_store.yaml

On Ubuntu CI, /bin/sh is dash, which interprets \n in echo — so this works in CI. However, on macOS or systems where bash is the default shell, echo outputs literal \n, producing:

project: feast_dbt_test\nregistry: data/registry.db\nprovider: local\nonline_store:\n  type: sqlite\n  path: data/online_store.db

This is a single line of invalid YAML, causing feast dbt import on the next recipe line to fail with a YAML parsing error. Using printf instead of echo is the portable fix, since printf always interprets escape sequences.

Impact: make test-python-integration-dbt fails on developer machines running macOS or any system where Make's shell is bash.

Suggested change
echo "project: feast_dbt_test\nregistry: data/registry.db\nprovider: local\nonline_store:\n type: sqlite\n path: data/online_store.db" > feast_repo/feature_store.yaml
printf 'project: feast_dbt_test\nregistry: data/registry.db\nprovider: local\nonline_store:\n type: sqlite\n path: data/online_store.db\n' > feast_repo/feature_store.yaml
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@cd sdk/python/tests/integration/dbt/feast_repo && \
echo "Testing feast dbt import..." && \
feast dbt import -m ../test_dbt_project/target/manifest.json -e driver_id -d file --tag feast && \
echo "Verifying Feast objects..." && \
feast feature-views list && \
feast entities list
@cd sdk/python && \
echo "Running pytest integration tests..." && \
python -m pytest tests/integration/dbt/test_dbt_integration.py -v --tb=short
@echo "✓ dbt integration tests completed successfully!"

test-python-universal-spark: ## Run Python Spark integration tests
PYTHONPATH='.' \
FULL_REPO_CONFIGS_MODULE=sdk.python.feast.infra.offline_stores.contrib.spark_repo_configuration \
Expand Down
4 changes: 3 additions & 1 deletion sdk/python/feast/dbt/codegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ def generate(
if not entity_cols:
raise ValueError("At least one entity column must be specified")

excluded = set(entity_cols) | {self.timestamp_field}
# Note: entity columns should NOT be excluded - FeatureView.__init__
# expects entity columns to be in the schema and will extract them
excluded = {self.timestamp_field}
if exclude_columns:
excluded.update(exclude_columns)

Expand Down
4 changes: 2 additions & 2 deletions sdk/python/feast/dbt/mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
# Mapping from FeastType to ValueType for entity value inference
FEAST_TYPE_TO_VALUE_TYPE: Dict[FeastType, ValueType] = {
String: ValueType.STRING,
Int32: ValueType.INT64,
Int32: ValueType.INT32,
Int64: ValueType.INT64,
Float32: ValueType.DOUBLE,
Float32: ValueType.FLOAT,
Float64: ValueType.DOUBLE,
Bool: ValueType.BOOL,
Bytes: ValueType.BYTES,
Expand Down
1 change: 1 addition & 0 deletions sdk/python/tests/integration/dbt/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Integration tests for dbt import functionality
32 changes: 32 additions & 0 deletions sdk/python/tests/integration/dbt/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""
Conftest for dbt integration tests.

This is a standalone conftest that doesn't depend on the main Feast test infrastructure.
"""

from pathlib import Path

import pytest

# This conftest is minimal and doesn't import the main feast conftest
# to avoid complex dependency chains for dbt-specific tests

# Path to the test dbt project manifest
TEST_DBT_PROJECT_DIR = Path(__file__).parent / "test_dbt_project"
TEST_MANIFEST_PATH = TEST_DBT_PROJECT_DIR / "target" / "manifest.json"


def pytest_collection_modifyitems(config, items): # noqa: ARG001
"""
Skip dbt integration tests if manifest.json doesn't exist.

These tests require running 'dbt build' first to generate the manifest.
The dbt-integration-test workflow handles this, but regular unit test
runs don't, so we skip them to avoid failures.
"""
if not TEST_MANIFEST_PATH.exists():
skip_marker = pytest.mark.skip(
reason="dbt manifest.json not found - run 'dbt build' first or use dbt-integration-test workflow"
)
for item in items:
item.add_marker(skip_marker)
9 changes: 9 additions & 0 deletions sdk/python/tests/integration/dbt/pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[pytest]
# Prevent loading parent conftest.py files which may import heavy dependencies
# like ray that are not needed for dbt integration tests
norecursedirs = ..
asyncio_mode = auto

# Test markers
markers =
dbt: dbt integration tests
Loading
Loading