Skip to content

chore(utils): add type hints to utils functions [python] - #4123

Open
yogeshwaran-c wants to merge 2 commits into
taskforcesh:masterfrom
yogeshwaran-c:feat/python-utils-type-hints
Open

chore(utils): add type hints to utils functions [python]#4123
yogeshwaran-c wants to merge 2 commits into
taskforcesh:masterfrom
yogeshwaran-c:feat/python-utils-type-hints

Conversation

@yogeshwaran-c

@yogeshwaran-c yogeshwaran-c commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds missing type hints to three functions in python/bullmq/utils.py:

  • isRedisVersionLowerThan(current_version: str | None, minimum_version: str) -> bool
  • extract_result(job_task: asyncio.Task, emit_callback: Callable[[str, Any], None]) -> Any
  • get_parent_key(opts: dict[str, str]) -> Optional[str]

Also adds the necessary imports (asyncio, Callable, Optional).

Note: isRedisVersionLowerThan now treats current_version=None as "not lower" (returns False) instead of raising. This is a small but intentional runtime change: callers like RedisConnection.version are typed as str | None and previously would have crashed if checked before getRedisVersion() ran.

Motivation

These functions previously lacked type annotations, while the rest of the file (e.g. parse_json_string_values, object_to_flat_array, is_redis_cluster, get_cluster_nodes, get_node_client) is fully typed. This change brings consistency and improves IDE support / static analysis. Apart from the documented None-guard above, no other runtime behavior is altered.

Test plan

  • No runtime behavior changes apart from the documented None-guard in isRedisVersionLowerThan
  • Existing Python tests continue to pass

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds missing type annotations to a few helper functions in python/bullmq/utils.py to improve consistency and IDE/static-analysis support in the Python BullMQ implementation.

Changes:

  • Added imports needed for typing (asyncio, Callable, Optional).
  • Added return/parameter type hints for isRedisVersionLowerThan, extract_result, and get_parent_key.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/bullmq/utils.py Outdated
Comment thread python/bullmq/utils.py Outdated
Comment thread python/bullmq/utils.py Outdated
Comment thread python/bullmq/utils.py Outdated
@roggervalf roggervalf changed the title feat(python): add type hints to utils functions chore(utils): add type hints to utils functions [python] Apr 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/bullmq/utils.py
Comment on lines +9 to 12
def isRedisVersionLowerThan(current_version: str | None, minimum_version: str) -> bool:
if current_version is None:
return False
return semver.Version.parse(current_version).compare(minimum_version) == -1
@yogeshwaran-c

Copy link
Copy Markdown
Contributor Author

Thanks for catching this! You're right that the current_version is None short-circuit is a small runtime behavior change, not purely an annotation change. I've updated the PR description to call this out explicitly.

The None-handling is intentional defensive behavior that matches the new current_version: str | None annotation: callers like RedisConnection.version start out as None until getRedisVersion() populates them, so returning False ("not lower than the minimum") is the safest default and avoids a TypeError from semver.Version.parse(None) if the check happens to run before the version is fetched. Treating an unknown version as "not lower" also matches the optimistic semantics the rest of the codebase assumes when version info isn't yet available.

Happy to split this into two commits (annotations-only + the None-guard) if you'd prefer that for review clarity.

@manast

manast commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread python/bullmq/utils.py


def extract_result(job_task, emit_callback):
def extract_result(job_task: asyncio.Future[Any], emit_callback: Callable[[str, Any], None]) -> Any:
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.

3 participants