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

Simplify registries: no prefixes, direct sub-registries access #315

Merged
merged 9 commits into from
Feb 20, 2020

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Feb 20, 2020

What do these changes do?

Refactor the registries massively, to prepare for even more registry and handler types.

Description

Originally, there were sub-registries and one operator registry consisting of sib-registries. And the operator registry was exposing multiple registed_blah_blah_handler(), [has|get|iter]_blah_blah_handlers() methods, so on.

As the number of handler type increases, having these proxying methods becomes a problem: they do nothing, just proxy to the actual sub-registries (simple lists), but bring a lot of method signatures that has to be maintained and tested.

With this PR, we remove all of these proxying methods completely (technically, deprecate them and warn if they are used, but keep them for a while until the next major release).

The sub-registries should now be used directly, and its handlers should be accesses via the similar methods (same-named, but different typing signatures: e.g. [get|iter]_handlers(...)).

Purpose: In the following PRs, even more registry and handler types will be added: e.g. daemons & timers. Not having even more proxying methods seems convenient.


Also, registries prefixes are removed. They were used for the sub-handlers only. Now, the prefixes are implemented directly for sub-handler decorators.

Since registries were never created directly by the users (unlike the accessing methods above), we have no need to keep backward compatible changes for deprecated prefixes — we just drop them.


The end users should not be affected (in theory).

Issues/PRs

Issues: #19

Related: #298

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

Handler id prefixes are used only for the sub-handlers
(e.g. "parent-id/child-id"), and sub-handlers is a rarely
used feature.

But the prefixes were implemented as a feature of registries.
Move the prefixing to the sub-handlers related code, and remove
prefixes from the registries. This should simplify their code.
In order to not get caught in a situation when an arg is forgotten,
which can cause misbehaviour or errors. The handlers are hidden
from the users, so we have no need to keep their interfaces simple.
@nolar nolar added the refactoring Code cleanup without new features added label Feb 20, 2020
@zincr
Copy link

zincr bot commented Feb 20, 2020

🤖 zincr found 0 problems , 1 warning

ℹ️ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Large Commits

Checks all commits for large additions to a single file. Large commits should be reviewed more carefully for potential copyright and licensing issues

This file contains a substantial change, please review to determine if the change comes from an external source and if there are any copyright or licensing issues to be aware of

@zincr
Copy link

zincr bot commented Feb 20, 2020

🤖 zincr found 1 problem , 1 warning

❌ 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
     

Large Commits

Checks all commits for large additions to a single file. Large commits should be reviewed more carefully for potential copyright and licensing issues

This file contains a substantial change, please review to determine if the change comes from an external source and if there are any copyright or licensing issues to be aware of

@nolar nolar marked this pull request as ready for review February 20, 2020 14:11
@nolar nolar merged commit 51a3a70 into zalando-incubator:master Feb 20, 2020
@nolar nolar deleted the simplify-registries-4 branch February 20, 2020 16:05
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