Skip to content

Commit

Permalink
fix: Fixed issues with Linting, unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Bhargav Dodla committed Jun 27, 2024
1 parent 0f52559 commit 59fa72a
Show file tree
Hide file tree
Showing 35 changed files with 1,304 additions and 508 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pr_local_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
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.repository == 'feast-dev/feast'
github.repository != 'feast-dev/feast'
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand Down
42 changes: 40 additions & 2 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
architecture: x64
- name: Setup Go
id: setup-go
uses: actions/setup-go@v2
with:
go-version: 1.19.7
- name: Install apache-arrow on ubuntu
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V "libarrow-dev=11.0.0-1"
sudo apt install -y -V pkg-config
- name: Install uv
run: |
curl -LsSf https://astral.sh/uv/install.sh | sh
Expand All @@ -37,18 +52,41 @@ jobs:
key: ${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-uv-${{ hashFiles(format('**/py{0}-ci-requirements.txt', env.PYTHON)) }}
- name: Install dependencies
run: make install-python-ci-dependencies-uv
- name: Compile Go along with Extensions
if: matrix.os == 'ubuntu-latest'
run: |
make install-go-proto-dependencies
make install-go-ci-dependencies
COMPILE_GO=true python setup.py develop
CGO_LDFLAGS_ALLOW=".*" COMPILE_GO=True python setup.py build_ext --inplace
- name: Test Milvus tests
if: matrix.os == 'ubuntu-latest'
run: python -m pytest -n 1 --color=yes sdk/python/tests/expediagroup/test_milvus_online_store.py
- name: Test Python
if: matrix.os == 'ubuntu-latest'
run: make test-python-unit
- name: Test Python for Mac
if: matrix.os != 'ubuntu-latest'
run: python -m pytest -n 8 --color=yes sdk/python/tests --ignore=sdk/python/tests/expediagroup --ignore=sdk/python/tests/integration/e2e/test_go_feature_server.py

unit-test-go:
runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
python-version: [ "3.9", "3.10", "3.11"]
os: [ ubuntu-latest ]
env:
OS: ${{ matrix.os }}
PYTHON: ${{ matrix.python-version }}
steps:
- uses: actions/checkout@v4
- name: Setup Python
id: setup-python
uses: actions/setup-python@v5
with:
python-version: "3.10"
python-version: ${{ matrix.python-version }}
architecture: x64
- name: Upgrade pip version
run: |
pip install --upgrade "pip>=22.1,<23"
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ celerybeat-schedule
# Environments
.env
.venv
.venv*
env/
venv/
ENV/
Expand Down
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ benchmark-python-local:
IS_TEST=True FEAST_IS_LOCAL_TEST=True python -m pytest --integration --benchmark --benchmark-autosave --benchmark-save-data sdk/python/tests

test-python-unit:
python -m pytest -n 1 --color=yes sdk/python/tests/expediagroup/test_milvus_online_store.py
python -m pytest -n 8 --color=yes sdk/python/tests --ignore=sdk/python/tests/expediagroup

test-python-integration:
Expand Down Expand Up @@ -409,10 +408,10 @@ install-go-ci-dependencies:
# The `go get` command on the previous lines download the lib along with replacing the dep to `feast-dev/gopy`
# but the following command is needed to install it for some reason.
go install github.com/go-python/gopy
python -m pip install "pybindgen==0.22.1" "protobuf<5,>3.20"
python -m pip install "pybindgen==0.22.1" "protobuf>=4.24.0,<5"

install-protoc-dependencies:
pip install --ignore-installed protobuf==4.24.0 "grpcio-tools>=1.56.2,<2" mypy-protobuf==3.1.0
pip install --ignore-installed "protobuf>=4.24.0,<5" "grpcio-tools>=1.56.2,<2" mypy-protobuf==3.1.0

compile-protos-go: install-go-proto-dependencies install-protoc-dependencies
python setup.py build_go_protos
Expand Down
4 changes: 2 additions & 2 deletions go/embedded/online_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ import (
"github.com/feast-dev/feast/go/internal/feast/onlineserving"
"github.com/feast-dev/feast/go/internal/feast/registry"
"github.com/feast-dev/feast/go/internal/feast/server"
"github.com/feast-dev/feast/go/internal/feast/server/healthcheck"
"github.com/feast-dev/feast/go/internal/feast/server/logging"
"github.com/feast-dev/feast/go/internal/feast/transformation"
"github.com/feast-dev/feast/go/protos/feast/serving"
prototypes "github.com/feast-dev/feast/go/protos/feast/types"
"github.com/feast-dev/feast/go/types"
jsonlog "github.com/rs/zerolog/log"
"google.golang.org/grpc/health"
"google.golang.org/grpc/health/grpc_health_v1"
)

Expand Down Expand Up @@ -311,7 +311,7 @@ func (s *OnlineFeatureService) StartGprcServerWithLogging(host string, port int,

grpcServer := grpc.NewServer()
serving.RegisterServingServiceServer(grpcServer, ser)
healthService := healthcheck.NewHealthChecker()
healthService := health.NewServer()
grpc_health_v1.RegisterHealthServer(grpcServer, healthService)

go func() {
Expand Down
28 changes: 0 additions & 28 deletions go/internal/feast/server/healthcheck/grpc-healthchecker.go

This file was deleted.

2 changes: 0 additions & 2 deletions go/internal/test/go_integration_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ func SetupCleanFeatureRepo(basePath string) error {
if err != nil {
return err
}

t := time.Now()

formattedTime := fmt.Sprintf("%d-%02d-%02dT%02d:%02d:%02d",
Expand All @@ -124,7 +123,6 @@ func SetupCleanFeatureRepo(basePath string) error {
if err != nil {
return err
}

return nil
}

Expand Down
4 changes: 1 addition & 3 deletions sdk/python/feast/embedded_go/online_features_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,7 @@ def transformation_callback(
full_feature_names: bool,
) -> int:
try:
odfv = fs.get_on_demand_feature_view(
on_demand_feature_view_name, allow_cache=True
)
odfv = fs.get_on_demand_feature_view(on_demand_feature_view_name)

input_record = pa.RecordBatch._import_from_c(input_arr_ptr, input_schema_ptr)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from datetime import timedelta
from typing import Dict, List, Literal, Optional, Union

from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict
from pydantic import Field as PydanticField
from typing_extensions import Annotated, Self

Expand Down Expand Up @@ -57,6 +57,8 @@ class RequestSourceModel(DataSourceModel):
Pydantic Model of a Feast RequestSource.
"""

model_config = ConfigDict(protected_namespaces=())

name: str
model_type: Literal["RequestSourceModel"] = "RequestSourceModel"
schema_: List[FieldModel]
Expand Down Expand Up @@ -106,6 +108,8 @@ class SparkSourceModel(DataSourceModel):
Pydantic Model of a Feast SparkSource.
"""

model_config = ConfigDict(protected_namespaces=())

name: str
model_type: Literal["SparkSourceModel"] = "SparkSourceModel"
table: Optional[str] = None
Expand Down Expand Up @@ -180,6 +184,8 @@ class PushSourceModel(DataSourceModel):
Pydantic Model of a Feast PushSource.
"""

model_config = ConfigDict(protected_namespaces=())

name: str
model_type: Literal["PushSourceModel"] = "PushSourceModel"
batch_source: AnyBatchDataSource
Expand Down Expand Up @@ -246,6 +252,8 @@ class KafkaSourceModel(DataSourceModel):
Pydantic Model of a Feast KafkaSource.
"""

model_config = ConfigDict(protected_namespaces=())

name: str
model_type: Literal["KafkaSourceModel"] = "KafkaSourceModel"
timestamp_field: str
Expand Down
17 changes: 8 additions & 9 deletions sdk/python/feast/expediagroup/pydantic_models/entity_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

from datetime import datetime
from json import dumps
from typing import Callable, Dict, Optional
from typing import Dict, Optional

from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict
from typing_extensions import Self

from feast.entity import Entity
Expand All @@ -21,6 +21,12 @@ class EntityModel(BaseModel):
Pydantic Model of a Feast Entity.
"""

model_config = ConfigDict(
arbitrary_types_allowed=True,
extra="allow",
json_encoders={ValueType: lambda v: int(dumps(v.value, default=str))},
)

name: str
join_key: str
value_type: Optional[ValueType] = None
Expand All @@ -30,13 +36,6 @@ class EntityModel(BaseModel):
created_timestamp: Optional[datetime] = None
last_updated_timestamp: Optional[datetime] = None

class Config:
arbitrary_types_allowed = True
extra = "allow"
json_encoders: Dict[object, Callable] = {
ValueType: lambda v: int(dumps(v.value, default=str))
}

def to_entity(self) -> Entity:
"""
Given a Pydantic EntityModel, create and return an Entity.
Expand Down
Loading

0 comments on commit 59fa72a

Please sign in to comment.