chore: Clean up makefile (feast-dev#4799)

* clean up dependency installation makefile tasks Signed-off-by: Matt Green <[email protected]> * rewrite lock-python-dependencies-all to be more DRY Signed-off-by: Matt Green <[email protected]> * update environment setup docs Signed-off-by: Matt Green <[email protected]> * update smoke test Signed-off-by: Matt Green <[email protected]> * small change Signed-off-by: Matt Green <[email protected]> --------- Signed-off-by: Matt Green <[email protected]> Signed-off-by: Theodor Mihalache <[email protected]>
tmihalac · Dec 3, 2024 · 8e1f940 · 8e1f940
1 parent a1bc04b
commit 8e1f940
Show file tree

Hide file tree

Showing 17 changed files with 84 additions and 94 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -23,7 +23,7 @@
 	// "forwardPorts": [],
 
 	// Uncomment the next line to run commands after the container is created.
-	// "postCreateCommand": "make install-python-ci-dependencies-uv-venv"
+	// "postCreateCommand": "make install-python-dependencies-dev"
 
 	// Configure tool-specific properties.
 	// "customizations": {},

diff --git a/.github/fork_workflows/fork_pr_integration_tests_aws.yml b/.github/fork_workflows/fork_pr_integration_tests_aws.yml
@@ -73,7 +73,7 @@ jobs:
             sudo apt update
             sudo apt install -y -V libarrow-dev
       - name: Install dependencies
-        run: make install-python-ci-dependencies-uv
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -85,5 +85,3 @@ jobs:
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "File and not Snowflake and not BigQuery and not minio_registry"
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "dynamo and not Snowflake and not BigQuery and not minio_registry"
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "Redshift and not Snowflake and not BigQuery and not minio_registry"
-
-
diff --git a/.github/fork_workflows/fork_pr_integration_tests_gcp.yml b/.github/fork_workflows/fork_pr_integration_tests_gcp.yml
@@ -75,7 +75,7 @@ jobs:
             sudo apt update
             sudo apt install -y -V libarrow-dev
       - name: Install dependencies
-        run: make install-python-ci-dependencies-uv
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -86,4 +86,3 @@ jobs:
         run: |
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "BigQuery and not dynamo and not Redshift and not Snowflake and not minio_registry"
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "File and not dynamo and not Redshift and not Snowflake and not minio_registry"
-
diff --git a/.github/fork_workflows/fork_pr_integration_tests_snowflake.yml b/.github/fork_workflows/fork_pr_integration_tests_snowflake.yml
@@ -65,7 +65,7 @@ jobs:
             sudo apt update
             sudo apt install -y -V libarrow-dev
       - name: Install dependencies
-        run: make install-python-ci-dependencies-uv
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -82,4 +82,3 @@ jobs:
         run: |
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "Snowflake and not dynamo and not Redshift and not Bigquery and not gcp and not minio_registry"
           pytest -n 8 --cov=./ --cov-report=xml --color=yes sdk/python/tests --integration --durations=5 --timeout=1200 --timeout_method=thread -k "File and not dynamo and not Redshift and not Bigquery and not gcp and not minio_registry"
-
diff --git a/.github/workflows/java_master_only.yml b/.github/workflows/java_master_only.yml
@@ -126,7 +126,7 @@ jobs:
           key: ${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-uv-${{ hashFiles(format('**/py{0}-ci-requirements.txt', env.PYTHON)) }}
 
       - name: Install Python dependencies
-        run: make install-python-ci-dependencies-uv
+        run: make install-python-dependencies-ci
       - uses: actions/cache@v4
         with:
           path: ~/.m2/repository

diff --git a/.github/workflows/java_pr.yml b/.github/workflows/java_pr.yml
@@ -180,7 +180,7 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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
+        run: make install-python-dependencies-ci
       - name: Run integration tests
         run:  make test-java-integration
       - name: Save report

diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml
@@ -19,6 +19,6 @@ jobs:
         run: curl -LsSf https://astral.sh/uv/install.sh | sh
       - name: Install dependencies
         run: |
-          make install-python-ci-dependencies-uv
+          make install-python-dependencies-ci
       - name: Lint python
         run: make lint-python
diff --git a/.github/workflows/master_only.yml b/.github/workflows/master_only.yml
@@ -65,7 +65,7 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -130,4 +130,4 @@ jobs:
           make push-${{ matrix.component }}-docker REGISTRY=${REGISTRY} VERSION=${GITHUB_SHA}
 
           docker tag ${REGISTRY}/${{ matrix.component }}:${GITHUB_SHA} ${REGISTRY}/${{ matrix.component }}:develop
-          docker push ${REGISTRY}/${{ matrix.component }}:develop
+          docker push ${REGISTRY}/${{ matrix.component }}:develop
diff --git a/.github/workflows/nightly-ci.yml b/.github/workflows/nightly-ci.yml
@@ -141,7 +141,7 @@ jobs:
         if: matrix.os == 'macos-13'
         run: brew install apache-arrow
       - name: Install dependencies
-        run: make install-python-ci-dependencies-uv
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -154,4 +154,4 @@ jobs:
           SNOWFLAKE_CI_PASSWORD: ${{ secrets.SNOWFLAKE_CI_PASSWORD }}
           SNOWFLAKE_CI_ROLE: ${{ secrets.SNOWFLAKE_CI_ROLE }}
           SNOWFLAKE_CI_WAREHOUSE: ${{ secrets.SNOWFLAKE_CI_WAREHOUSE }}
-        run: make test-python-integration
+        run: make test-python-integration
diff --git a/.github/workflows/pr_integration_tests.yml b/.github/workflows/pr_integration_tests.yml
@@ -86,7 +86,7 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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
+        run: make install-python-dependencies-ci
       - name: Setup Redis Cluster
         run: |
           docker pull vishnunair/docker-redis-cluster:latest
@@ -100,4 +100,4 @@ jobs:
           SNOWFLAKE_CI_PASSWORD: ${{ secrets.SNOWFLAKE_CI_PASSWORD }}
           SNOWFLAKE_CI_ROLE: ${{ secrets.SNOWFLAKE_CI_ROLE }}
           SNOWFLAKE_CI_WAREHOUSE: ${{ secrets.SNOWFLAKE_CI_WAREHOUSE }}
-        run: make test-python-integration
+        run: make test-python-integration
diff --git a/.github/workflows/pr_local_integration_tests.yml b/.github/workflows/pr_local_integration_tests.yml
@@ -48,7 +48,7 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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
+        run: make install-python-dependencies-ci
       - name: Test local integration tests
         if: ${{ always() }}  # this will guarantee that step won't be canceled and resources won't leak
         run: make test-python-integration-local
diff --git a/.github/workflows/smoke_tests.yml b/.github/workflows/smoke_tests.yml
@@ -33,6 +33,8 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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-dependencies-uv
+        run: |
+          uv pip sync --system sdk/python/requirements/py${{ matrix.python-version }}-requirements.txt
+          uv pip install --system --no-deps .
       - name: Test Imports
-        run: python -c "from feast import cli"
+        run: python -c "from feast import cli"
diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml
@@ -36,7 +36,7 @@ jobs:
           path: ${{ steps.uv-cache.outputs.dir }}
           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
+        run: make install-python-dependencies-ci
       - name: Test Python
         run: make test-python-unit
 

diff --git a/.gitpod.yml b/.gitpod.yml
@@ -8,7 +8,7 @@ tasks:
       uv pip install pre-commit
       pre-commit install --hook-type pre-commit --hook-type pre-push
       source .venv/bin/activate
-      export PYTHON=3.10 && make install-python-ci-dependencies-uv-venv
+      export PYTHON=3.10 && make install-python-dependencies-dev
     #   git config --global alias.ci 'commit -s'
     #   git config --global alias.sw switch
     #   git config --global alias.st status

diff --git a/Makefile b/Makefile
@@ -23,6 +23,12 @@ endif
 TRINO_VERSION ?= 376
 PYTHON_VERSION = ${shell python --version | grep -Eo '[0-9]\.[0-9]+'}
 
+PYTHON_VERSIONS := 3.9 3.10 3.11
+define get_env_name
+$(subst .,,py$(1))
+endef
+
+
 # General
 
 format: format-python format-java
@@ -35,50 +41,50 @@ protos: compile-protos-python compile-protos-docs
 
 build: protos build-java build-docker
 
-# Python SDK
+# Python SDK - local
+# formerly install-python-ci-dependencies-uv-venv
+# editable install
+install-python-dependencies-dev:
+	uv pip sync sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
+	uv pip install --no-deps -e .
 
-install-python-dependencies-uv:
-	uv pip sync --system sdk/python/requirements/py$(PYTHON_VERSION)-requirements.txt
-	uv pip install --system --no-deps .
+# Python SDK - system
+# the --system flag installs dependencies in the global python context
+# instead of a venv which is useful when working in a docker container or ci.
 
-install-python-dependencies-uv-venv:
-	uv pip sync sdk/python/requirements/py$(PYTHON_VERSION)-requirements.txt
-	uv pip install --no-deps .
+# Used in github actions/ci
+# formerly install-python-ci-dependencies-uv
+install-python-dependencies-ci:
+	uv pip sync --system sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
+	uv pip install --system --no-deps -e .
 
+# Used by multicloud/Dockerfile.dev
 install-python-ci-dependencies:
 	python -m piptools sync sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
 	pip install --no-deps -e .
 
-install-python-ci-dependencies-uv:
-	uv pip sync --system sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
-	uv pip install --system --no-deps -e .
-
-install-python-ci-dependencies-uv-venv:
-	uv pip sync sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
-	uv pip install --no-deps -e .
-
-lock-python-ci-dependencies:
-	uv pip compile --system --no-strip-extras setup.py --extra ci --output-file sdk/python/requirements/py$(PYTHON_VERSION)-ci-requirements.txt
-
-compile-protos-python:
-	python infra/scripts/generate_protos.py
-
+# Currently used in test-end-to-end.sh
 install-python:
 	python -m piptools sync sdk/python/requirements/py$(PYTHON_VERSION)-requirements.txt
 	python setup.py develop
 
-lock-python-dependencies:
-	uv pip compile --system --no-strip-extras setup.py --output-file sdk/python/requirements/py$(PYTHON_VERSION)-requirements.txt
-
 lock-python-dependencies-all:
-	# Remove all existing requirements because we noticed the lock file is not always updated correctly. Removing and running the command again ensures that the lock file is always up to date.
-	rm -r sdk/python/requirements/*
-	pixi run --environment py39 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.9 --system --no-strip-extras setup.py --output-file sdk/python/requirements/py3.9-requirements.txt"
-	pixi run --environment py39 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.9 --system --no-strip-extras setup.py --extra ci --output-file sdk/python/requirements/py3.9-ci-requirements.txt"
-	pixi run --environment py310 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.10 --system --no-strip-extras setup.py --output-file sdk/python/requirements/py3.10-requirements.txt"
-	pixi run --environment py310 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.10 --system --no-strip-extras setup.py --extra ci --output-file sdk/python/requirements/py3.10-ci-requirements.txt"
-	pixi run --environment py311 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.11 --system --no-strip-extras setup.py --output-file sdk/python/requirements/py3.11-requirements.txt"
-	pixi run --environment py311 --manifest-path infra/scripts/pixi/pixi.toml "uv pip compile -p 3.11 --system --no-strip-extras setup.py --extra ci --output-file sdk/python/requirements/py3.11-ci-requirements.txt"
+	# Remove all existing requirements because we noticed the lock file is not always updated correctly.
+	# Removing and running the command again ensures that the lock file is always up to date.
+	rm -rf sdk/python/requirements/* 2>/dev/null || true
+
+	$(foreach ver,$(PYTHON_VERSIONS),\
+		pixi run --environment $(call get_env_name,$(ver)) --manifest-path infra/scripts/pixi/pixi.toml \
+			"uv pip compile -p $(ver) --system --no-strip-extras setup.py \
+			--output-file sdk/python/requirements/py$(ver)-requirements.txt" && \
+		pixi run --environment $(call get_env_name,$(ver)) --manifest-path infra/scripts/pixi/pixi.toml \
+			"uv pip compile -p $(ver) --system --no-strip-extras setup.py --extra ci \
+			--output-file sdk/python/requirements/py$(ver)-ci-requirements.txt" && \
+	) true
+
+
+compile-protos-python:
+	python infra/scripts/generate_protos.py
 
 benchmark-python:
 	IS_TEST=True python -m pytest --integration --benchmark  --benchmark-autosave --benchmark-save-data sdk/python/tests

diff --git a/docs/how-to-guides/customizing-feast/adding-a-new-offline-store.md b/docs/how-to-guides/customizing-feast/adding-a-new-offline-store.md
@@ -440,11 +440,10 @@ test-python-universal-spark:
 
 ### 7. Dependencies
 
-Add any dependencies for your offline store to our `sdk/python/setup.py` under a new `<OFFLINE_STORE>__REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default. You will need to regenerate our requirements files. To do this, create separate pyenv environments for python 3.8, 3.9, and 3.10. In each environment, run the following commands:
+Add any dependencies for your offline store to our `sdk/python/setup.py` under a new `<OFFLINE_STORE>__REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default. You will need to regenerate our requirements files:
 
 ```
-export PYTHON=<version>
-make lock-python-ci-dependencies
+make lock-python-ci-dependencies-all
 ```
 
 ### 8. Add Documentation

diff --git a/docs/project/development-guide.md b/docs/project/development-guide.md
@@ -54,8 +54,8 @@ See [Contribution process](./contributing.md) and [Community](../community.md) f
 ## Making a pull request
 We use the convention that the assignee of a PR is the person with the next action.
 
-If the assignee is empty it means that no reviewer has been found yet. 
-If a reviewer has been found, they should also be the assigned the PR. 
+If the assignee is empty it means that no reviewer has been found yet.
+If a reviewer has been found, they should also be the assigned the PR.
 Finally, if there are comments to be addressed, the PR author should be the one assigned the PR.
 
 PRs that are submitted by the general public need to be identified as `ok-to-test`. Once enabled, [Prow](https://github.com/kubernetes/test-infra/tree/master/prow) will run a range of tests to verify the submission, after which community members will help to review the pull request.
@@ -120,51 +120,39 @@ Note that this means if you are midway through working through a PR and rebase,
 
 ## Feast Python SDK and CLI
 ### Environment Setup
-Setting up your development environment for Feast Python SDK and CLI:
-1. Ensure that you have Docker installed in your environment. Docker is used to provision service dependencies during testing, and build images for feature servers and other components.
+#### Tools
+- Docker:  Docker is used to provision service dependencies during testing, and build images for feature servers and other components.
    - Please note that we use [Docker with BuiltKit](https://docs.docker.com/develop/develop-images/build_enhancements/).
    - _Alternatively_ - To use [podman](https://podman.io/) on a Fedora or RHEL machine, follow this [guide](https://github.com/feast-dev/feast/issues/4190)
-2. Ensure that you have `make` and Python (3.9 or above) installed.
-3. _Recommended:_ Create a virtual environment to isolate development dependencies to be installed
-  ```sh
-  # create & activate a virtual environment
-  python -m venv venv/
-  source venv/bin/activate
-  ```
-4. (M1 Mac only): Follow the [dev guide](https://github.com/feast-dev/feast/issues/2105)
-5. Install uv. It is recommended to use uv for managing python dependencies.
+- `make` is used to run various scripts
+- [uv](https://docs.astral.sh/) for managing python dependencies. [installation instructions](https://docs.astral.sh/uv/getting-started/installation/)
+- (M1 Mac only): Follow the [dev guide if you have issues](https://github.com/feast-dev/feast/issues/2105)
+- (Optional): Node & Yarn (needed for building the feast UI)
+- (Optional): [Pixi](https://pixi.sh/latest/) for recompile python lock files. Only when you make changes to requirements or simply want to update python lock files to reflect latest versioons.
+
+### Quick start
+- create a new virtual env: `uv venv --python 3.11` (Replace the python version with your desired version)
+- activate the venv: `source venv/bin/activate`
+- Install dependencies `make install-python-dependencies-dev`
+
+### building the UI
 ```sh
-curl -LsSf https://astral.sh/uv/install.sh | sh
-```
-or
-```ssh
-pip install uv
-```
-6. (Optional): Install Node & Yarn. Then run the following to build Feast UI artifacts for use in `feast ui`
-```
 make build-ui
 ```
-7. (Optional) install pixi. pixi is necessary to run step 8 for all python versions at once.
-```sh
-curl -fsSL https://pixi.sh/install.sh | bash
-```
-8. (Optional): Recompile python lock files. Only when you make changes to requirements or simply want to update python lock files to reflect latest versioons.
-```sh
-make lock-python-dependencies-all
-``` 
-9. Install development dependencies for Feast Python SDK and CLI. This will install package versions from the lock file, install editable version of feast and compile protobufs.
 
-If running inside a virtual environment:
+### Recompiling python lock files
+Recompile python lock files. This only needs to be run when you make changes to requirements or simply want to update python lock files to reflect latest versions.
+
 ```sh
-make install-python-ci-dependencies-uv-venv
+make lock-python-dependencies-all
 ```
 
-Otherwise:
+### Building protos
 ```sh
-make install-python-ci-dependencies-uv
+make compile-protos-python
 ```
 
-10. Spin up Docker Image
+### Building a docker image for development
 ```sh
 docker build -t docker-whale -f ./sdk/python/feast/infra/feature_servers/multicloud/Dockerfile .
 ```
@@ -405,7 +393,7 @@ It will:
 
 ### Testing with Github Actions workflows
 
-Please refer to the maintainers [doc](maintainers.md) if you would like to locally test out the github actions workflow changes. 
+Please refer to the maintainers [doc](maintainers.md) if you would like to locally test out the github actions workflow changes.
 This document will help you setup your fork to test the ci integration tests and other workflows without needing to make a pull request against feast-dev master.
 
 ## Feast Data Storage Format
@@ -414,4 +402,3 @@ Feast data storage contracts are documented in the following locations:
 
 * [Feast Offline Storage Format](https://github.com/feast-dev/feast/blob/master/docs/specs/offline_store_format.md): Used by BigQuery, Snowflake \(Future\), Redshift \(Future\).
 * [Feast Online Storage Format](https://github.com/feast-dev/feast/blob/master/docs/specs/online_store_format.md): Used by Redis, Google Datastore.
-