Test PR #2
Test PR #2
Conversation
|
@sourcery-ai review |
Reviewer's GuideThis PR enhances the Makefile with modular preflight checks, tightens and upgrades project dependencies in pyproject.toml, applies minor code refactors for clarity, and updates test modules along with the lock file. Flow Diagram for Updated Makefile Target Dependencies and New Preflight Checksgraph TD
subgraph New Preflight Checks Introduced
CUV["check-uv (new)"]
CD["check-docker (new)"]
CDC["check-docker-compose (new)"]
end
S["setup (modified)"]
TI["test-integration (modified)"]
BDP["build-deb-package (modified)"]
S -- "now depends on" --> CUV
TI -- "now depends on" --> CDC
CDC -- "depends on" --> CD
BDP -- "now depends on" --> CD
BDP -- "depends on" --> S
classDef new_node fill:#ccffcc,stroke:#333,stroke-width:2px
classDef modified_node fill:#ffffcc,stroke:#333,stroke-width:2px
class CUV,CD,CDC new_node
class S,TI,BDP modified_node
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Alex-Burmak - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- An upper version pin ('< 8.2') was added for 'click', which violates the instruction not to pin upper versions. (link)
- An upper version pin ('< 9.0') was added for 'deepdiff', which is not allowed per the review instruction. (link)
- An upper version pin ('< 2.30') was added for 'requests', which violates the instruction. (link)
- An upper version pin ('< 1.6') was added for 'mypy', which is not allowed per the review instruction. (link)
- An upper version pin ('< 4.0') was added for 'pylint', which violates the instruction. (link)
General comments:
- In pyproject.toml you’ve relaxed constraints on deepdiff, pyyaml, and requests—please verify their updated APIs don’t break any existing code paths.
- You introduced CLICKHOUSE_VERSION in the Makefile but it isn’t referenced by any targets—either wire it into your build/test steps or remove it to avoid confusion.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🔴 Review instructions: 5 blocking issues
- 🟡 Testing: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "python-dateutil", | ||
| "pyyaml < 5.4", | ||
| "requests < 2.30", | ||
| "pyyaml >= 5.4", |
There was a problem hiding this comment.
suggestion (bug_risk): Add upper bound for PyYAML
Capping the major version (e.g., <6.0) helps prevent issues from future breaking changes in PyYAML.
| "pyyaml >= 5.4", | |
| "pyyaml >= 5.4, < 6.0", |
| dev = [ | ||
| "behave", | ||
| "docker < 8.0", | ||
| "docker-compose < 1.29 ; python_version < '3.13'", |
There was a problem hiding this comment.
suggestion (testing): Remove version qualifiers for pyfakefs
Unrestricted pyfakefs versions may introduce breaking changes in tests. Please specify an upper version limit or use a known stable version to ensure test reliability.
| "docker-compose < 1.29 ; python_version < '3.13'", | |
| "pyfakefs >= 5.5.0, < 6.0.0", |
| dependencies = [ | ||
| "boto3 ~= 1.23.10", | ||
| "click ~= 8.0.4", | ||
| "click >= 8.1, < 8.2", |
There was a problem hiding this comment.
issue (review_instructions): An upper version pin ('< 8.2') was added for 'click', which violates the instruction not to pin upper versions.
Please remove the upper version bound for 'click' to comply with the instruction against pinning upper versions. Only lower bounds or compatible release specifiers (e.g., ~=) are allowed.
Review instructions:
Path patterns: pyproject.toml
Instructions:
Do not suggest to pin upper versions
| "click >= 8.1, < 8.2", | ||
| "cloup", | ||
| "deepdiff < 6.4.0", | ||
| "deepdiff >= 8.0, < 9.0", |
There was a problem hiding this comment.
issue (review_instructions): An upper version pin ('< 9.0') was added for 'deepdiff', which is not allowed per the review instruction.
Please remove the upper version bound for 'deepdiff' to comply with the instruction not to pin upper versions.
Review instructions:
Path patterns: pyproject.toml
Instructions:
Do not suggest to pin upper versions
| "pyyaml < 5.4", | ||
| "requests < 2.30", | ||
| "pyyaml >= 5.4", | ||
| "requests >= 2.20, < 2.30", |
There was a problem hiding this comment.
issue (review_instructions): An upper version pin ('< 2.30') was added for 'requests', which violates the instruction.
Please remove the upper version bound for 'requests' to comply with the instruction not to pin upper versions.
Review instructions:
Path patterns: pyproject.toml
Instructions:
Do not suggest to pin upper versions
| "ruff", | ||
| "isort >= 6.0; python_version >= '3.9'", | ||
| "isort >= 6.0", | ||
| "mypy >= 1.5, < 1.6", |
There was a problem hiding this comment.
issue (review_instructions): An upper version pin ('< 1.6') was added for 'mypy', which is not allowed per the review instruction.
Please remove the upper version bound for 'mypy' to comply with the instruction not to pin upper versions.
Review instructions:
Path patterns: pyproject.toml
Instructions:
Do not suggest to pin upper versions
| "isort >= 6.0; python_version >= '3.9'", | ||
| "isort >= 6.0", | ||
| "mypy >= 1.5, < 1.6", | ||
| "pylint >= 3.0, < 4.0", |
There was a problem hiding this comment.
issue (review_instructions): An upper version pin ('< 4.0') was added for 'pylint', which violates the instruction.
Please remove the upper version bound for 'pylint' to comply with the instruction not to pin upper versions.
Review instructions:
Path patterns: pyproject.toml
Instructions:
Do not suggest to pin upper versions
1 participant
Summary by Sourcery
Add environment variable and prerequisite checks in Makefile, bump Python and dependency version requirements, and apply minor code formatting tweaks
Enhancements: