Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR] Rebalance the modules to reduce complexity #298

Closed
5 tasks done
kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Closed
5 tasks done

[PR] Rebalance the modules to reduce complexity #298

kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Labels
archive refactoring Code cleanup without new features added

Comments

@kopf-archiver
Copy link

kopf-archiver bot commented Aug 18, 2020

A pull request by nolar at 2020-01-24 10:29:31+00:00
Original URL: zalando-incubator/kopf#298
Merged by nolar at 2020-01-29 13:51:22+00:00

What do these changes do?

Move classes and functions across the modules, extract new modules — in order to reduce the complexity and prepare the codebase for adding even more code.

The behaviour is not changed, only the imports.

Description

Before this PR, some modules were too large, up to 1k of lines, containing too many aspects and topics — as it has historically happened when new features were added. For example, activity execution was part of handling.py, as it was nearly a clone of handler execution. Similarly, event processing and handler invocation were there too (in handling.py) — since the times they were short and simple.

With this PR, some huge modules are split into smaller ones, and classes/functions are moved all around. For example, watch-event handling is now renamed to processing and moved to processing.py — as per event processing of the event-driven design, which it is by its nature — this releases the term handling to the callback functions, which are currently called handlers.

This is needed to make more space for adding more code (e.g. for daemons and timers) without exploding the modules beyond understanding.


See the list of commits for individual changes (they are all atomic). Briefly:

  • Rename watch-event "handling" routines to "processing" routines.
  • Extract the in-memory multi-step handling cycle to a reusable routine (used in activities, will be used in daemons/timers).
  • Split handling.py -> handling.py + processing.py + activities.py
  • Split registries.py -> registries.py + handlers.py + callbacks.py + errors.py

The latter split also removes the cyclic imports as detected by LGTM (all 16 alarms at once) — this cycle is broken by moving BaseHandler away from registries:

  • registries
    • invocation (for get_kwargs(), needed for matching callbacks)
      • lifecycles (for LifeCycleFn, needed for Invokable definition)
        • registries (for BaseHandler, needed for handlers kwarg)

Replaced with a regular dependency tree:

  • registries
    • invocation (for get_kwargs(), needed for matching callbacks)
      • lifecycles (for LifeCycleFn, needed for Invokable definition)
        • handlers (for BaseHandler, needed for handlers kwarg)
    • handlers (for all handlers dataclasses, previously located in registries)

The users should not be affected, as these are the internal changes only — unless they monkey-patch some of the Kopf's internals.

Issues/PRs

Issues: #19

Related: #315

Type of changes

  • Refactoring (non-breaking change which does not alter the behaviour)

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
@kopf-archiver kopf-archiver bot closed this as completed Aug 18, 2020
@kopf-archiver kopf-archiver bot changed the title [archival placeholder] [PR] Rebalance the modules to reduce complexity Aug 19, 2020
@kopf-archiver kopf-archiver bot added the refactoring Code cleanup without new features added label Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive refactoring Code cleanup without new features added
Projects
None yet
Development

No branches or pull requests

0 participants