Migrate from flake8/black/isort to Ruff#663
Conversation
5a78179 Merge pull request intelowlproject#223 from certego/develop 76df2c2 added ruff and fixed a bug (intelowlproject#221) 59bfe83 Explicitly disabled "xpack.security" in Elasticsearch container 0c262e2 Updated CHANGELOG 0599640 Fixed create_python_cache workflow (intelowlproject#222) 4f21023 Added documentation - part 2 (intelowlproject#220) 0d2f931 updated github actions versions (intelowlproject#218) 013f31a Python caching revisited (intelowlproject#217) 548235b Linter requirements reconciliated (intelowlproject#215) b6fd709 Updated changelog 0cfa137 Ecr (intelowlproject#201) ed2dd16 Updated codeQL action to v3 (intelowlproject#216) 5f44be8 APT caching revisited (intelowlproject#214) cf7c16d Updated linters and added changes detection exclusions (intelowlproject#213) a492676 Deprecation of license check `table-headers` (intelowlproject#212) 0a6db48 Updated python linters also in '_python.yml' workflow git-subtree-dir: .github git-subtree-split: 5a78179ab0cbea826c416f8975251b519c2541fc
- Updated .github subtree from 76df2c2 to v2.0.0 (commit 5a78179) - Added Ruff linter and formatter support - Enabled use_ruff_formatter and use_ruff_linter in CI - Disabled old linters (black, isort, flake8) in favor of Ruff - Updated Python linter dependencies - Merged changelogs from both repos
- Removed black, isort, flake8, pylint, bandit, autoflake from pre-commit - Ruff provides equivalent functionality for all of these - Faster pre-commit execution - Avoids conflicting linter rules
- Replaced black and isort badges with Ruff badge in README - Updated PR template checklist to mention Ruff instead of Black/Flake/Isort
- Fixed 37 import sorting and unused import issues - Reformatted 5 files with ruff format - Fixed pre-commit ruff args (removed invalid 'check' argument) - 14 wildcard import warnings remain (F403) which require manual review
- Exclude Ruff's cache directory from version control
As per maintainer feedback, silenced F403 warnings for wildcard imports in __init__.py files since they are acceptable for this project.
- Created pyproject.toml to extend ruff config for easier CLI usage - Fixed import sorting in 5 Django migration files - Now 'ruff check .' works without explicit --config flag
- Reformatted 30 Python files with ruff format - This is the result of running 'ruff format .' after creating pyproject.toml - No logic changes, only formatting (line breaks, spacing)
The new certego/.github v2.0.0 APT cache workflow requires a packages file, even if empty. This is a workaround for the workflow's strict validation that was introduced in v2.0.0.
Pass packages.txt to the workflow to fix APT cache restoration step
Remove comments that were being interpreted as package names by apt-get
Use rabbitmq:4-management-alpine to fix Docker image pull error. The default 'latest' is not a valid tag for management-alpine images.
Use version '4' instead of '4-management-alpine' since the workflow automatically appends '-management-alpine' suffix
|
@mlodic This PR is now ready for review! CI Compatibility FixesDuring testing, I encountered several compatibility issues with the certego/.github v2.0.0 update that weren't present in previous versions. Here's what I fixed: Issues Found & Resolved:1. APT Cache Workflow Issue (Commits: d5794f5, 03a9614, 20e5c01)
2. RabbitMQ Image Tag Issue (Commits: 62ab9b5, c6ce0ef)
Why These Issues Occurred:These are new requirements in certego/.github v2.0.0 that weren't present in the previous version used by GreedyBear. The old workflow didn't have strict APT caching validation or automatic RabbitMQ tag suffixing. Result:✅ All 277 tests passing |
mlodic
left a comment
There was a problem hiding this comment.
hey thanks for the detailed PR!
could you also please create a separate PR to update the docs? https://github.com/intelowlproject/docs/blob/main/docs/GreedyBear/Contribute.md
| "F", | ||
| # Isort - https://docs.astral.sh/ruff/rules/#isort-i | ||
| "I" | ||
| ] |
There was a problem hiding this comment.
can you please comment these like this contributor did here and also include the missing modules?
There was a problem hiding this comment.
Thanks for the feedback! I'll update the Ruff configuration to include the missing rule modules and add proper documentation.
Changes Made:
Added Rule Modules:
- E/W - Full pycodestyle error and warning coverage (currently only have E4, E7, E9)
- N - pep8-naming (variable/function naming conventions)
- UP - pyupgrade (modern Python syntax)
- B - flake8-bugbear (common Python bugs)
- C4 - flake8-comprehensions (list/dict comprehension improvements)
- DJ - flake8-django (Django-specific linting)
All rules are commented with links to official docs.
Auto-Fixed Violations (43 total):
- Type annotations (
List[str]→list[str],Optional[X]→X | None) - Unused imports
- Unnecessary future imports
- Redundant file mode arguments
- Unnecessary dict comprehensions
- Missing
strict=inzip()calls
Added to Ignore List (intentional patterns):
- B006/B008/B017/B023/B904 - Test helper patterns (mutable defaults, function calls in defaults, exception handling)
- C401/C408 - Code style preferences (dict() calls, generators)
- DJ001/DJ008/DJ012 - Django patterns (null on CharField, models without
__str__, field ordering) - E501 - Long lines in docstrings
- N801/N802/N803/N804/N806/N818 - Existing naming conventions (viewType, iocType, ML variables like X_train, Django test classmethods)
- UP008/UP031 - Test code clarity (explicit super(), old-style formatting)
These ignores preserve intentional code patterns while still catching real issues.
Note: 16 files changed due to automated modernizations from the new rules. All changes are safe improvements without logic changes.
There was a problem hiding this comment.
I appreciate the fast and clear update, thank you.
I think we could get this opportunity to leverage the new rules to cleanup some of the old code, even if that require manual intervention. In this way we enforce best practices in the future too.
In particular, all the U,N,E,C stuff are style based and should be easy to align to the right and modern standards without touching the logic. So I would err to the side of avoid adding all those exclusions because, I mean, we are updating the linters to get better code, so we should listen to them.
Can you try to apply those changes? If B/D changes are too difficult, feel free to skip
There was a problem hiding this comment.
Thanks for the guidance! I'd like to clarify the scope before proceeding.
I analyzed the E/N/UP/C violations and found:
Easy fixes (~15 violations):
- N804: Rename
self→clsin test classmethods (8 files) - UP008: Simplify
super()calls (3 occurrences) - UP031: Modernize string formatting (1 occurrence)
- C401: Fix set comprehension (1 occurrence)
Invasive changes (~12 violations):
- N801:
viewType→ViewType,iocType→IocType(used throughout codebase) - N802: Rename migration functions
generalHoneypot,migrateData,removeDdospot(Django migrations - risky!) - N803/N806: ML variable naming
X,X_train,X_test→x,x_train,x_test(breaks ML conventions)
Question: Should I:
- Option A: Fix the easy ones only (~15 violations) in this PR - keeps changes focused and create a follow-up issue for the invasive naming changes (viewType, migrations) to be handled separately
- Option B: Fix everything in this PR despite the large scope (+2,223 −515 already)
The viewType/iocType renaming alone would touch ~50+ files across the codebase. Happy to proceed with whichever approach you prefer!
There was a problem hiding this comment.
B (bugbear) and DJ (Django) Changes
As suggested, the B and DJ violations require significant refactoring, so I'll defer them to a separate issue/PR:
B (flake8-bugbear) - Separate PR:
- B006/B008: Mutable defaults in test helpers - requires refactoring test utility functions
- B017: Blind exception catching - needs specific exception types identified
- B023: Loop variable in lambda - requires function refactoring
- B904: Raise without
from- needs exception chaining analysis
DJ (flake8-django) - Separate PR:
- DJ001:
null=Trueon CharField - requires database migration + API contract review - DJ008: Models without
__str__- needs__str__methods added to legacy models - DJ012: Django field ordering - requires reordering fields in existing models
These need more extensive testing/review and are better handled separately to keep this PR focused on the linter migration. I'll create a follow-up issue to track them after this merges.
Added comprehensive Ruff rule modules as requested: - E/W: Full pycodestyle error and warning coverage - N: pep8-naming for naming conventions - UP: pyupgrade for modern Python syntax - B: flake8-bugbear for common Python bugs - C4: flake8-comprehensions for list/dict improvements - DJ: flake8-django for Django-specific linting All rules are documented with inline comments and links to official docs. Fixed 43 auto-fixable violations (imports, annotations, etc). Added comprehensive ignore list for intentional code patterns: - Test helpers (mutable defaults, classmethods) - ML conventions (X, X_train naming) - Django patterns (null=True on CharField, models without __str__) - Legacy naming (viewType, iocType, migration functions) All checks passing ✅
Applied Ruff auto-fixes for straightforward style improvements: - N804: Renamed 'self' → 'cls' in test classmethods (6 occurrences) - UP008: Simplified super() calls (3 occurrences) - UP031/UP032: Modernized string formatting (2 occurrences) - C401: Fixed set comprehensions (7 occurrences) Total: 18 violations fixed across 7 files. Invasive changes (N801/N802 model/function renames, N803/N806 ML naming) deferred to follow-up issue for dedicated testing and review.
|
Fixed the easy violations in commit b8c314c:
Total: 18 violations fixed across 7 files. Follow-up PlanI'll create separate issues/PRs after this merges:
|
- Update Code Style section to reflect Ruff migration from black/isort/flake8 - Add detailed Ruff capabilities and rule sets documentation - Add manual Ruff command examples for contributors - Add missing Cowrie Session API endpoint documentation - Add comprehensive Serializers section (EnrichmentSerializer, FeedsRequestSerializer, etc.) - Add Validation Functions section (feed_type_validation, ordering_validation) - Improve organization with clear section headers - Fix naming consistency (general_honeypot_list → General Honeypot List) Related: intelowlproject/GreedyBear#663
Description
This PR migrates GreedyBear from the old linters (black, isort, flake8) to Ruff. The changes include:
.githubsubtree to certego/.github v2.0.0 which includes Ruff supportMost of the file changes (76 files: +1,737, -378) are from the subtree update. The actual GreedyBear code changes are minimal (formatting and import sorting).
Related issues
Closes #640
Type of change
Checklist
develop.Ruff) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.