Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
3 changes: 0 additions & 3 deletions .flake8

This file was deleted.

4 changes: 4 additions & 0 deletions .github/workflows/check-pr-title.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ on:
- reopened
- synchronize

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
check:
name: Check PR Title
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/github-stale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ on:
schedule:
- cron: "0 */5 * * *"

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
stale:
runs-on: ubuntu-latest
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/test-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ name: E2E Test
on:
pull_request

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
e2e-test:
name: E2E Test
Expand Down
17 changes: 14 additions & 3 deletions .github/workflows/test-python.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,27 @@ on:
push:
branches: [main]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
Comment on lines +8 to +10
Copy link
Member

Choose a reason for hiding this comment

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

IIRC, we discussed before that this part is not needed given that we don't have limitations on concurrent runs due to GitHub enterprise.

Copy link
Contributor

Choose a reason for hiding this comment

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

A use case is re-pushing the branch for the PR while the workflow is running. Even with no limitations, is that useful to keep the existing run going?

Copy link
Member

@andreyvelich andreyvelich Aug 4, 2025

Choose a reason for hiding this comment

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

Might be useful to compare results, but I am fine to cancel them for now.
@kramaranya Please can you apply the change to all GitHub action tests in that case ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, updated in 4853da4


jobs:
test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.9', '3.11']

name: Test (Python ${{ matrix.python-version }})

steps:
- uses: actions/checkout@v4

- name: Set up Python
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: '3.9'
python-version: ${{ matrix.python-version }}

- name: Verify
run: make verify
Expand All @@ -26,4 +37,4 @@ jobs:
- name: Upload coverage to Coveralls
uses: coverallsapp/github-action@v2
with:
github-token: ${{ secrets.GITHUB_TOKEN }} # Pass the GITHUB_TOKEN
github-token: ${{ secrets.GITHUB_TOKEN }}
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ cd python
pip install -e .
```

Install development tools:
#### Development Build (Optional)
To install development tools and the latest API modules directly from the master branch:
```sh
pip install pytest black isort flake8 coverage pre-commit
pip install -e ".[dev]"
```

## Development Workflow
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ uv-venv:
echo "uv virtual environment already exists in $(VENV_DIR)."; \
fi

# make test-unit will produce html coverage by default. Run with `make test-unit report=xml` to produce xml report.
# make test-python will produce html coverage by default. Run with `make test-python report=xml` to produce xml report.
.PHONY: test-python
test-python: uv-venv
@uv pip install "./python[test]"
@uv pip install --project $(PY_DIR) -e "$(PY_DIR)[dev]"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this --project flag ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it for project isolation, but since we have only one project right now, I can remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 542ddcd

@uv run coverage run --source=kubeflow.trainer.api.trainer_client,kubeflow.trainer.utils.utils -m pytest ./python/kubeflow/trainer/api/trainer_client_test.py
@uv run coverage report -m kubeflow/trainer/api/trainer_client.py kubeflow/trainer/utils/utils.py
ifeq ($(report),xml)
Expand Down
2 changes: 1 addition & 1 deletion python/kubeflow/trainer/api/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# flake8: noqa
# ruff: noqa

# import apis into api package

12 changes: 6 additions & 6 deletions python/pyproject.toml
Copy link
Member

Choose a reason for hiding this comment

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

Let's also migrate to the official package as part of this PR: https://pypi.org/project/kubeflow-trainer-api/

Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ classifiers = [
dependencies = [
"kubernetes>=27.2.0",
"pydantic>=2.10.0",
"kubeflow_trainer_api@git+https://github.com/kubeflow/trainer.git@master#subdirectory=api/python_api"
"kubeflow-trainer-api>=2.0.0",
]
[project.optional-dependencies]
test = [
dev = [
"pytest>=7.0",
"pytest-mock>=3.10",
"coverage>=7.0",
"kubeflow_trainer_api@git+https://github.com/kubeflow/trainer.git@master#subdirectory=api/python_api",
"ruff>=0.12.2",
"pre-commit>=4.2.0",
]

[project.urls]
Expand All @@ -52,10 +55,7 @@ line-length = 100
select = ["E", "F", "W"]
ignore = ["E203"]

[dependency-groups]
dev = [
"ruff>=0.12.2",
]


[tool.uv]
package = true
Expand Down
108 changes: 95 additions & 13 deletions python/uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.