Skip to content

refactor: extract helpers to reduce pylint complexity#834

Open
renansuaris wants to merge 1 commit into
redis:mainfrom
renansuaris:refactor/too-many-statements
Open

refactor: extract helpers to reduce pylint complexity#834
renansuaris wants to merge 1 commit into
redis:mainfrom
renansuaris:refactor/too-many-statements

Conversation

@renansuaris
Copy link
Copy Markdown

@renansuaris renansuaris commented May 26, 2026

What does this change do?

Refactors the too-many-statements violations reported by Pylint in large methods across the codebase by extracting parts of the logic into smaller private helpers.

Why is this needed?

The affected methods were handling too many responsibilities in a single block, which made them harder to read, test, and maintain. Splitting the logic into smaller helpers reduces complexity while keeping the external behavior unchanged.

What changed?

  • Internal logic was split into smaller helper functions
  • Main methods remain as orchestrators
  • No intentional behavior changes
  • Migration flow and metrics were preserved

How was it tested?

  • Existing tests continue to pass.

Notes for reviewers

  • The refactor was kept minimal and focused only on the reported cases

Note

Medium Risk
Touches RediSearch query building and model metaclass/schema paths where regressions would affect all queries and indexing, though the PR is refactor-only and claims unchanged behavior.

Overview
This PR splits oversized methods into private helpers to satisfy Pylint complexity limits, without changing intended behavior.

In datetime_migration, HashModel processing is refactored so key scanning, byte normalization, and per-key datetime conversion live in _collect_hash_keys, _normalize_hash_data, and _process_hash_key, with _process_hash_model only batching and delegating.

DataMigrator.run_migrations_with_monitoring now uses _log_pending_migrations, _build_monitoring_result, and _log_monitoring_summary for listing, result dict assembly, and verbose output (including dry-run).

In model.py, resolve_value delegates to type-specific _resolve_* helpers; query execute is split into _build_execute_args, _execute_command, and _parse_execute_results; ModelMeta.__new__ and JSON index schema_for_type recursion are broken into static/instance helpers with the same orchestration flow.

Reviewed by Cursor Bugbot for commit b80c1a6. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented May 26, 2026

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit b80c1a6. Configure here.

monitor=monitor,
errors=[],
dry_run=True,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dry-run result now includes extra success_rate field

Low Severity

The dry-run path now returns a success_rate key in its result dictionary that wasn't present before the refactoring. The original dry-run result contained only applied_count, total_migrations, performance_stats, errors, and dry_run. By routing both the dry-run and normal paths through _build_monitoring_result, the dry-run result now also includes success_rate, changing the API contract.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b80c1a6. Configure here.

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.

1 participant