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

CI and development tools are not aligned #124

Open
2 tasks
dmartinol opened this issue Feb 6, 2025 · 0 comments
Open
2 tasks

CI and development tools are not aligned #124

dmartinol opened this issue Feb 6, 2025 · 0 comments

Comments

@dmartinol
Copy link

System Info

not relevant

Information

  • The official example scripts
  • My own modified scripts

🐛 Describe the bug

According to CONTRIBUTING.md, these tools are used at development time:

rye run pytest
rye run lint
rye run format

All these tools apply on the entire codebase, so they currently detect issues in different files after a fresh clone of the repo. See Error logs section.

On the contrary, the CI builds a Pre-commit job that uses a different set of tools configured in .pre-commit-config.yaml, which only apply to the ^src/llama_stack_client/lib/.* path.
The CI does not detect any error, so we're allowing to commit changes with code issues that are not identified by the CI job.

Error logs

% rye run format                                   
513 files left unchanged
error: Failed to parse script.py:3:17: missing closing quote in string literal
README.md:56: code block parse error Command '['/Users/dmartino/projects/AI/instructlab/llama-stack-client-python/.venv/bin/python', '-m', 'ruff', 'format', '--stdin-filename=script.py', '--line-length=100']' returned non-zero exit status 2.
error: script failed with exit status: 1
% rye run lint
src/llama_stack_client/__init__.py:10:5: F811 Redefinition of unused `Transport` from line 4
   |
 8 |     Stream,
 9 |     Timeout,
10 |     Transport,
   |     ^^^^^^^^^ F811
11 |     AsyncClient,
12 |     AsyncStream,
   |
   = help: Remove definition: `Transport`

src/llama_stack_client/_base_client.py:1174:35: B006 Do not use mutable data structures for argument defaults
...
% rye run pytest         
ImportError while loading conftest '/Users/dmartino/projects/AI/instructlab/llama-stack-client-python/tests/conftest.py'.
tests/conftest.py:8: in <module>
    from pytest_asyncio import is_async_test
E   ImportError: cannot import name 'is_async_test' from 'pytest_asyncio' (/Users/dmartino/projects/AI/instructlab/llama-stack-client-python/.venv/lib/python3.11/site-packages/pytest_asyncio/__init__.py)

Expected behavior

  • Development and CI tools should produce the same result.
  • Either we extend the scope of CI to match the dev tools, or reduce the scope of dev tools to match the CI configuration.
  • First option is preferred but requires an initial code cleanup to fix all the issues (or configure the tools to skip the "accepted" issues).
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

No branches or pull requests

1 participant