Skip to content

chore: enable mypy v2 pre-commit hook; fix all lint/type errors it surfaces#31

Open
Siddharth Mitra (sidmitra) wants to merge 15 commits into
mainfrom
SM-mypy-v2
Open

chore: enable mypy v2 pre-commit hook; fix all lint/type errors it surfaces#31
Siddharth Mitra (sidmitra) wants to merge 15 commits into
mainfrom
SM-mypy-v2

Conversation

@sidmitra
Copy link
Copy Markdown
Contributor

@sidmitra Siddharth Mitra (sidmitra) commented May 13, 2026

Summary

Uncomments and upgrades the mirrors-mypy pre-commit hook from the previously disabled v0.902 stub to v2.1.0, then fixes every error surfaced by running it and the full pre-commit suite.


mypy v2 — config

8 options new in mypy v2 added to [tool.mypy] (all at their defaults). Removed show_none_errors which was deleted in v2.

Per-module ignore_missing_imports overrides added for confluent_kafka and cotyledon — neither package ships type stubs or py.typed in the mypy environment.


mypy v2 — source fixes

File Error Fix
brokers/base.py Stale # type: ignore on poll() abstract method Removed
brokers/__init__.py BaseProducer not in __all__ (implicit_reexport=false) Added
brokers/dummy.py Missing return None in poll() (warn_no_return) Added explicit return None
brokers/kafka.py _consumer typed as CConsumer but initialised to None Annotated as CConsumer | None
brokers/kafka.py commit(message=) gets str | bytes | Message | None, expects Message cast("CMessage", message) after None check
bus.py EventJsonEncoder.default() unannotated Added (self, o: Any) -> Any
bus.py self.producer typed as bare None Annotated as BaseProducer | None
cli.py super().__init__(worker_id) passes int, expects WorkerId WorkerId(worker_id) via from cotyledon.types import WorkerId
cli.py class Worker(cotyledon.Service) subclasses Any-typed base # type: ignore[misc] on class definition

pylint v4 — fixes

File Code Fix
brokers/base.py R0917 Made flush/on_delivery/fail_silently keyword-only
brokers/dummy.py R1711 Moved disable-next=useless-return to def line
brokers/kafka.py E0712 # pylint: disable=catching-non-exception on except KafkaError
bus.py R0912 # pylint: disable=too-many-branches on receive()

ruff S101 — assert replacements

assert is banned outside tests by ruff rule S101. Replaced three bare asserts in brokers/kafka.py with explicit exceptions:

Location Replacement
Consumer.poll / Consumer.ack — consumer-not-open guard raise RuntimeError("Consumer is not open. Use as a context manager.")
Producer.produce — value type narrowing raise TypeError(f"Expected str or bytes, got {type(value).__name__}")

Message as CMessage was also moved from the runtime import to the TYPE_CHECKING block, since it is only referenced in string-form cast() calls (fixing pylint W0611 unused-import).

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 13, 2026

⚠️ Snyk checks are incomplete.

Status Scan Engine Critical High Medium Low Total (0)
⚠️ Open Source Security 0 0 0 0 See details
⚠️ Licenses 0 0 0 0 See details
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@sidmitra Siddharth Mitra (sidmitra) force-pushed the SM-mypy-v2 branch 2 times, most recently from 646a4e5 to f8dd194 Compare May 13, 2026 20:56
@sidmitra Siddharth Mitra (sidmitra) changed the title chore: enable mypy pre-commit hook at v2.1.0 chore: enable mypy v2 pre-commit hook, fix all resulting lint errors May 13, 2026
Uncomment and upgrade the mirrors-mypy pre-commit hook from the
previously commented-out v0.902 stub to v2.1.0.
pyproject.toml:
- Remove show_none_errors (dropped in mypy v2)

eventbusk/brokers/base.py:
- Remove stale # type: ignore on poll() and produce() abstract methods

eventbusk/brokers/dummy.py:
- Add missing 'return None' in Consumer.poll()

eventbusk/bus.py:
- Add 'from typing import Any' and annotate EventJsonEncoder.default()
- Annotate self.producer as BaseProducer | None (Producer is a factory
  function, not a class; use the base type for the annotation)
- Import BaseProducer from .brokers
- Remove stale # type: ignore on event_type(**event_data)

eventbusk/brokers/__init__.py:
- Export BaseProducer in __all__ so bus.py import resolves cleanly
New options made explicit (all at their defaults):
  allow_redefinition_old = false
  extra_checks = false
  follow_untyped_imports = false
  no_strict_bytes = false
  report_deprecated_as_note = false
  show_error_code_links = false
  show_error_end = false
  strict_equality_for_none = false

Also fix double-space typo in local_partial_types key.
pyproject.toml:
- Remove suggestion-mode (dropped in pylint v4)

eventbusk/brokers/__init__.py:
- Add pylint: disable=invalid-name on Consumer/Producer factory aliases
  (PascalCase names are intentional public API, not classes)

eventbusk/brokers/base.py + dummy.py + kafka.py:
- Make flush/on_delivery/fail_silently keyword-only args in produce()
  to satisfy R0917 too-many-positional-arguments; all callers already
  used keyword syntax so no call-site changes needed

eventbusk/brokers/dummy.py:
- Move pylint: disable=useless-return to def line (R1711 is reported
  there); keep explicit return None to satisfy mypy warn_no_return

eventbusk/brokers/kafka.py:
- Add pylint: disable=catching-non-exception on except KafkaError
- Add pylint: disable=bad-exception-cause on raise ProducerError from exc
  (KafkaError is a C-extension type; pylint can't verify its hierarchy)

eventbusk/bus.py:
- Annotate HooksT with TypeAlias and add inline invalid-name disable
  (union alias HooksT not recognised as TypeAlias by pylint naming check)
- Add pylint: disable=too-many-branches on wrapper() (16 branches, limit 15)
- Add public before_receive_hooks / after_receive_hooks properties

tests/test_brokers.py:
- Add pylint: disable=raising-non-exception on raise KafkaError(...)

tests/test_bus.py:
- Use public before_receive_hooks / after_receive_hooks properties
- Simplify empty-list checks to 'not bus.before/after_receive_hooks'
- Add pylint: disable=no-value-for-parameter on foo_processor() calls
  (decorator transforms signature; pylint doesn't see the wrapper)
Move inline disable comments to pylint: disable-next= on preceding
line to keep lines within the 88-character flake8 limit.
Copy link
Copy Markdown

Copilot AI left a comment

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 enables the mypy v2 pre-commit hook (upgrading from the previously commented-out stub) and updates code/config to satisfy the stricter type-checking and linting results across the project.

Changes:

  • Enable and pin mirrors-mypy v2.1.0 in pre-commit and update mypy/pylint configuration accordingly.
  • Tighten typing in the event bus and broker interfaces (e.g., producer type, JSON encoder typing, broker API signatures).
  • Adjust broker modules/exports to satisfy typing and lint rules.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pyproject.toml Updates mypy configuration for v2 (removes deprecated options, adds explicit new defaults).
eventbusk/bus.py Adds type annotations/properties and tweaks event reconstruction / producer typing.
eventbusk/brokers/base.py Updates abstract broker interfaces (removes stale ignores; adjusts produce signature).
eventbusk/brokers/__init__.py Exports BaseProducer via __all__ for typed imports.
eventbusk/brokers/dummy.py Adds explicit return None in poll() to satisfy mypy/pylint.
eventbusk/brokers/kafka.py Adds targeted pylint suppression on exception chaining line.
.pre-commit-config.yaml Enables mypy hook and pins to v2.1.0.

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

Comment thread eventbusk/bus.py Outdated
Comment thread eventbusk/bus.py Outdated
Comment thread eventbusk/bus.py Outdated
Comment on lines +278 to +281
# TODO: Fix following
# Too many arguments for "Event" [call-arg]
event = event_type(**event_data) # type: ignore
event.event_id = event_id
event = event_type(**event_data)
setattr(event, "event_id", event_id)
Both packages now ship py.typed and full stubs, making the blanket
# type: ignore comments on their imports unused. Removing them exposed
real type errors that are fixed here:

brokers/base.py:
  - Drop # type: ignore on 'from confluent_kafka import cimpl'

brokers/kafka.py:
  - Drop # type: ignore on confluent_kafka import block
  - Import Message as CMessage and cast from typing (needed for fixes below)
  - _consumer: CConsumer | None = None (was missing | None)
  - __exit__: guard close() with 'if self._consumer is not None'
  - poll/ack: assert self._consumer is not None (documents context-manager invariant)
  - ack: cast(CMessage, message) so commit(message=) gets the right type
  - produce: assert isinstance(value, (str, bytes)) to narrow away cimpl.Message;
    CProducer.produce() only accepts str | bytes
  - Drop stale # pylint: disable=bad-exception-cause; pylint 4.0.5 with
    --extension-pkg-whitelist resolves KafkaException's hierarchy correctly

cli.py:
  - Drop # type: ignore on 'import cotyledon' and class Worker definition
  - Import WorkerId from cotyledon.types; pass WorkerId(worker_id) to super().__init__()
  - self.name assignment: # type: ignore[misc] (intentional instance-level
    shadow of cotyledon.Service's ClassVar[str])
@sidmitra Siddharth Mitra (sidmitra) changed the title chore: enable mypy v2 pre-commit hook, fix all resulting lint errors chore: enable mypy v2 pre-commit hook; fix all type errors it surfaces May 14, 2026
- Replace assert guards in Consumer.poll/ack with RuntimeError and in
  Producer.produce with TypeError (ruff S101; assert is banned outside tests)
- Move `Message as CMessage` to TYPE_CHECKING block — it is only
  referenced in string-form cast() calls so pylint flagged it as unused
- Add mypy per-module overrides (ignore_missing_imports) for
  confluent_kafka and cotyledon — neither ships stubs in the mypy env,
  contradicting the earlier PR assumption
- Move `# type: ignore[misc]` from self.name assignment to the class
  definition line in cli.py, where mypy actually fires (subclassing Any)
- Wrap long logger.exception() call in bus.py (ruff E501 / ruff-format)
@sidmitra Siddharth Mitra (sidmitra) changed the title chore: enable mypy v2 pre-commit hook; fix all type errors it surfaces chore: enable mypy v2 pre-commit hook; fix all lint/type errors it surfaces May 15, 2026
Comment thread eventbusk/brokers/kafka.py Outdated
# Trigger any available delivery report callbacks from previous produce
self._producer.poll(0)
if not isinstance(value, (str, bytes)):
raise TypeError(f"Expected str or bytes, got {type(value).__name__}")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the type hint is wrong here. It should be str/bytes. since we do json.dumps

Message is not true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants