Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Move handler structures to kopf.structs package #332

Merged
merged 4 commits into from
Mar 26, 2020

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Mar 25, 2020

What do these changes do?

Yet another refactoring PR: just the moves of the code across modules, and some renames. No logic is changed in any way.

Description

It is done as a preparation for both the daemons (#330) and configurable state persistence (#331).

The main change: the handlers are now moved from kopf.reactor to kopf.structs — as they are no more than structs.

This allows to keep other structs (such as daemons or configs) to be also stored in kopf.structs instead of leaking into kopf.reactor and exploding it. The main different is that the kopf.reactor is for the processes and logic, while kopf.structs are for data structures with little or no logic (both reactor-involved and generic).

There are no user-facing changes or risks.

Issues/PRs

Related: #330 #331

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

Once the handlers were moved away into its own separate module,
it makes no sense to keep the types of some of the handlers' fields
in other modules (such as causation).

Instead, it should be all together with handlers — and so we can move
the whole thing to the `kopf.structs` package eventually (next commit).
Because all of them are structs after all, not the reactor's processes.
Because they are structs, after all. Also, in the few following
commits, the daemons structs will be introduced, and they need
to import and use handlers, from which they were spawned. It would
be problematic to import a high-level `reactor.handlers` from
a low-level `structs.containers` (if not moved).
Originally, the hope was to reuse them in daemons & timers. But this
breaks the semantics of the handlers: patches should be applied
on every exit, not only on successes; multiple restarts should
be counted & limited and distributed with backoffs; extra daemon
restarts are not needed at all if regular retrying works; etc.
@nolar nolar added the refactoring Code cleanup without new features added label Mar 25, 2020
@nolar nolar requested a review from haikoschol March 25, 2020 12:23
@zincr
Copy link

zincr bot commented Mar 25, 2020

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Mar 25, 2020

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@nolar nolar merged commit aa80d5a into zalando-incubator:master Mar 26, 2020
@nolar nolar deleted the classes-moved branch March 26, 2020 15:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactoring Code cleanup without new features added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants