Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove (direct) use of constantly #12607

Closed
4 tasks
DMRobertson opened this issue May 2, 2022 · 1 comment · Fixed by #12624
Closed
4 tasks

Remove (direct) use of constantly #12607

DMRobertson opened this issue May 2, 2022 · 1 comment · Fixed by #12624
Labels
good first issue Good for newcomers T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@DMRobertson
Copy link
Contributor

The entire source tree uses constantly in exactly one file, originally added in #6377.

from constantly import NamedConstant, Names

class EventRedactBehaviour(Names):
"""
What to do when retrieving a redacted event from the database.
"""
AS_IS = NamedConstant()
REDACT = NamedConstant()
BLOCK = NamedConstant()

In other places, we use the stdlib enum module instead. For instance:

class MatchChange(Enum):
no_change = auto()
now_true = auto()
now_false = auto()

I would like us to use the enum approach and ditch constantly, for three reasons:

  1. consistency across the source tree.
  2. mypy knows what an enum is; it don't know what a NamedConstant is. E.g. reveal_type(EventRedactBehaviour.AS_IS) yields synapse/handlers/message.py:1407: note: Revealed type is "Any".
  3. I prefer using a well-known tool from the stdlib rather than the lesser-known constantly.

Task list:

  • replace import of constantly with equivalent imports from stdlib enum
  • redefine the EventRedactBehaviour type in terms of enum
  • remove these lines from mypy.ini.
  • run the linter script: in particular, run mypy. Fix any errors that show up.
@DMRobertson DMRobertson added good first issue Good for newcomers T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels May 2, 2022
@andrewdoh
Copy link
Contributor

andrewdoh commented May 4, 2022

hi, id like to work on this. please check this PR out I made for the outlined above. @DMRobertson
#12624

@DMRobertson DMRobertson linked a pull request May 4, 2022 that will close this issue
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants