Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move Makefile to root directory #283

Merged
merged 4 commits into from
Feb 2, 2025
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
11 changes: 3 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ jobs:
- name: Cache dependencies
uses: Swatinem/rust-cache@v2
- name: Rust unit tests with coverage report
# https://github.com/xd009642/tarpaulin/issues/1092#issuecomment-1407739176
run: cargo tarpaulin --engine llvm --no-dead-code --no-fail-fast --all-features --workspace -o xml --output-dir ./cov-reports --skip-clean
- name: Upload coverage report
uses: actions/upload-artifact@v4
Expand All @@ -91,21 +90,17 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
cache: pip
cache-dependency-path: pyproject.toml
cache-dependency-path: python/pyproject.toml
- name: Setup Python venv
working-directory: ./python
run: |
make setup-venv
source venv/bin/activate
make develop
- name: Python unit tests with coverage report
run: |
pushd python
source venv/bin/activate
coverage run --include 'hudi/*' -m pytest -v
# move to parent so the reported file paths will match the actual repo paths
popd
coverage xml --data-file=python/.coverage -o ./cov-reports/cov-report-python-tests-${{ join(matrix.*, '-') }}.xml
coverage run --include 'python/hudi/*' -m pytest -v
coverage xml --data-file=.coverage -o ./cov-reports/cov-report-python-tests-${{ join(matrix.*, '-') }}.xml
- name: Upload coverage report
uses: actions/upload-artifact@v4
with:
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,22 @@ jobs:
uses: apache/skywalking-eyes/[email protected]

- name: Check rust code style
run: cd python && make check-rust
run: make check-rust

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: '3.9'
cache: pip
cache-dependency-path: pyproject.toml
cache-dependency-path: python/pyproject.toml

- name: Install python linter dependencies
working-directory: ./python
run: |
make setup-venv
source venv/bin/activate
pip install ruff==0.5.2 mypy==1.10.1

- name: Check python code style
working-directory: ./python
run: |
source venv/bin/activate
make check-python
26 changes: 8 additions & 18 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ view, instead, they will be linked to the corresponding issues.

## Commonly used dev commands

For most of the time, use dev commands specified in [`python/Makefile`](./python/Makefile), it applies to both Python
and Rust modules. You don't need to `cd` to the root directory and run `cargo` commands.
For most of the time, use dev commands specified in the [`Makefile`](Makefile).

To setup python virtual env, run

Expand All @@ -59,13 +58,10 @@ make setup-venv
```

> [!NOTE]
> This will run `python` command to setup the virtual environment. You can either change that to `python3.X`,
> or simply alias `python` to your local `python3.X` installation, for example:
> ```shell
> echo "alias python=/Library/Frameworks/Python.framework/Versions/3.12/bin/python3" >> ~/.zshrc`
> ```
> This will run `python3` command to set up the virtual environment in `venv/`.
> Activate the virtual environment by running `source venv/bin/activate` for example.

Once activate virtual env, build the project for development by
Once a virtual environment is activated, build the project for development by

```shell
make develop
Expand Down Expand Up @@ -96,24 +92,18 @@ For Python,
# For all tests
make test-python
# or
pytest -s
pytest -s python/tests

# For a specific test case
pytest tests/test_table_read.py -s -k "test_read_table_has_correct_schema"
pytest python/tests/test_table_read.py -s -k "test_read_table_has_correct_schema"
```

## Before creating a pull request

Run test commands to make sure the code is working as expected:
Run the below command and fix issues if any:

```shell
make test
```

Run check commands and follow the suggestions to fix the code:

```shell
make check
make format check test
```

## Create a pull request
Expand Down
23 changes: 13 additions & 10 deletions python/Makefile → Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,19 @@
# specific language governing permissions and limitations
# under the License.

SHELL := /bin/bash

.DEFAULT_GOAL := help

VENV := venv
MATURIN_VERSION := $(shell grep 'requires =' pyproject.toml | cut -d= -f2- | tr -d '[ "]')
PYTHON_DIR = python
MATURIN_VERSION := $(shell grep 'requires =' $(PYTHON_DIR)/pyproject.toml | cut -d= -f2- | tr -d '[ "]')
PACKAGE_VERSION := $(shell grep version Cargo.toml | head -n 1 | awk '{print $$3}' | tr -d '"' )

.PHONY: setup-venv
setup-venv: ## Setup the virtualenv
$(info --- Setup virtualenv ---)
python -m venv $(VENV)
python3 -m venv $(VENV)

.PHONY: setup
setup: ## Setup the requirements
Expand All @@ -34,12 +37,12 @@ setup: ## Setup the requirements
.PHONY: build
build: setup ## Build Python binding of hudi-rs
$(info --- Build Python binding ---)
maturin build $(MATURIN_EXTRA_ARGS)
maturin build $(MATURIN_EXTRA_ARGS) -m $(PYTHON_DIR)/Cargo.toml

.PHONY: develop
develop: setup ## Install Python binding of hudi-rs
$(info --- Develop with Python binding ---)
maturin develop --extras=devel,pandas $(MATURIN_EXTRA_ARGS)
maturin develop --extras=devel,pandas $(MATURIN_EXTRA_ARGS) -m $(PYTHON_DIR)/Cargo.toml

.PHONY: format
format: format-rust format-python ## Format Rust and Python code
Expand All @@ -52,26 +55,26 @@ format-rust: ## Format Rust code
.PHONY: format-python
format-python: ## Format Python code
$(info --- Format Python code ---)
ruff format .
ruff format $(PYTHON_DIR)

.PHONY: check
check: check-rust check-python ## Run check on Rust and Python

.PHONY: check-rust
check-rust: ## Run check on Rust
$(info --- Check Rust clippy ---)
cargo clippy --all-targets --all-features --workspace -- -D warnings
cargo clippy --all-targets --all-features --workspace --no-deps -- -D warnings
$(info --- Check Rust format ---)
cargo fmt --all -- --check

.PHONY: check-python
check-python: ## Run check on Python
$(info --- Check Python format ---)
ruff format --check --diff .
ruff format --check --diff $(PYTHON_DIR)
$(info --- Check Python linting ---)
ruff check .
ruff check $(PYTHON_DIR)
$(info --- Check Python typing ---)
mypy .
pushd $(PYTHON_DIR); mypy .; popd

.PHONY: test
test: test-rust test-python ## Run tests on Rust and Python
Expand All @@ -84,4 +87,4 @@ test-rust: ## Run tests on Rust
.PHONY: test-python
test-python: ## Run tests on Python
$(info --- Run Python tests ---)
pytest -s
pytest -s $(PYTHON_DIR)
2 changes: 1 addition & 1 deletion demo/run_demo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fi

# install dependencies and run the demo apps
docker compose exec -T runner /bin/bash -c "
cd /opt/hudi-rs/python && \
cd /opt/hudi-rs && \
make setup develop && \
cd /opt/hudi-rs/demo/sql-datafusion && ./run.sh &&\
cd /opt/hudi-rs/demo/table-api-python && ./run.sh && \
Expand Down
6 changes: 3 additions & 3 deletions python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
# under the License.

import os
import pathlib
import zipfile
from pathlib import Path

import pytest

Expand All @@ -34,7 +34,7 @@ def _extract_testing_table(zip_file_path, target_path) -> str:
]
)
def get_sample_table(request, tmp_path) -> str:
fixture_path = "tests/table"
fixture_path = pathlib.Path(__file__).parent.joinpath("table")
table_name = request.param
zip_file_path = Path(fixture_path).joinpath(f"{table_name}.zip")
zip_file_path = pathlib.Path(fixture_path).joinpath(f"{table_name}.zip")
return _extract_testing_table(zip_file_path, tmp_path)
Loading