Skip to content

ci: add GitHub Actions workflow for lint and smoke test#309

Merged
Sunrisea merged 3 commits intonacos-group:masterfrom
cxhello:ci/add-github-actions
Mar 12, 2026
Merged

ci: add GitHub Actions workflow for lint and smoke test#309
Sunrisea merged 3 commits intonacos-group:masterfrom
cxhello:ci/add-github-actions

Conversation

@cxhello
Copy link
Contributor

@cxhello cxhello commented Mar 11, 2026

What is the purpose of the change

Closes #305

Add a lightweight CI pipeline to catch code-breaking issues before merge. Based on the direction approved by @Sunrisea in #305, and informed by industry research on similar SDK projects (boto3, dubbo-python, openai-python, nacos-sdk-go).

Brief changelog

ci: add GitHub Actions workflow for lint and smoke test

  • lint job: ruff check with pyflakes rules (F) on v2/ directory, ignoring F401 (unused imports) for now, excluding auto-generated grpcauto/ directory
  • smoke-test job: Python 3.9/3.11/3.13 matrix, install dependencies and verify module imports (import v2.nacos, conditionally import nacos on feature/v2)
  • Both branches (master and feature/v2) use the same workflow file

Note: A companion PR with identical content targets feature/v2.

Add CI pipeline with two jobs:
- lint: ruff check with pyflakes rules (F) on v2/ directory, ignoring
  F401 (unused imports) for now, excluding auto-generated grpcauto directory
- smoke-test: Python 3.9/3.11/3.13 matrix, verify module imports

Closes nacos-group#305

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
@Sunrisea
Copy link
Member

Thanks for the contribution!

One issue I noticed after merging: the two target branches (master and feature/v2) have different minimum Python version requirements, but the workflow uses the same matrix ['3.9', '3.11', '3.13'] for both.

Branch dependency analysis

Branch Min Python Key constraints
master (v3) 3.10+ a2a>=0.44, a2a-sdk>=0.3.20 require Python 3.10+
feature/v2 (v2) 3.9+ aiohttp>=3.10.11, pydantic>=2.10.4 require Python 3.9+

Impact

  • On master: the smoke-test job with Python 3.9 will fail at pip install -r requirements.txt because a2a / a2a-sdk packages do not support 3.9.
  • On feature/v2: the matrix ['3.9', '3.11', '3.13'] is correct and should work fine.

Suggested fix

Since this PR has already been merged to master, a follow-up PR is needed to adjust the matrix for master. For example:

# master branch: update to match Python 3.10+ requirement
python-version: ['3.10', '3.12', '3.13']

Both branches use PEP 604 union type syntax (X | None) which requires
Python 3.10+. Update smoke-test matrix to ['3.10', '3.12', '3.13'].

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
@cxhello
Copy link
Contributor Author

cxhello commented Mar 12, 2026

Thanks for the review! You're right about the Python version issue.

Updated the smoke-test matrix from ['3.9', '3.11', '3.13'] to ['3.10', '3.12', '3.13']. The root cause is that the codebase uses PEP 604 union type syntax (X | None), which requires Python 3.10+.

All CI checks are passing now. Will submit a follow-up PR to fix the same issue on feature/v2 as well.

@Sunrisea
Copy link
Member

Thanks for the review! You're right about the Python version issue.

Updated the smoke-test matrix from ['3.9', '3.11', '3.13'] to ['3.10', '3.12', '3.13']. The root cause is that the codebase uses PEP 604 union type syntax (X | None), which requires Python 3.10+.

All CI checks are passing now. Will submit a follow-up PR to fix the same issue on feature/v2 as well.

LGTM! The Python matrix update to ['3.10', '3.12', '3.13'] looks correct for master.

One minor suggestion: since this workflow file will also exist on feature/v2 (via #310), you may want to differentiate the trigger branches between the two PRs:

This avoids the situation where a push to feature/v2 triggers the master version of the workflow (with 3.10+ matrix), skipping the 3.9 coverage that feature/v2 actually supports. Each branch should only trigger its own CI config.

Each branch maintains its own CI config with branch-specific triggers
to avoid cross-branch workflow interference.

Signed-off-by: cxhello <caixiaohuichn@gmail.com>
@Sunrisea
Copy link
Member

LGTM

@cxhello
Copy link
Contributor Author

cxhello commented Mar 12, 2026

Good point! Updated both PRs to scope trigger branches separately:

Each branch now only triggers its own CI config.

@Sunrisea Sunrisea merged commit 12d7cff into nacos-group:master Mar 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add GitHub Actions CI for lint and smoke test

2 participants