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

Uniformize spam-checker API, part 1: the Code enum. #12703

Merged
merged 3 commits into from
May 20, 2022
Merged

Uniformize spam-checker API, part 1: the Code enum. #12703

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4657d28
Introduce an enum `Code` to replace the namespace class `Codes`.
Yoric May 20, 2022
5e38930
WIP: Applied feedback
Yoric May 20, 2022
0aeff5e
WIP: Simplifying Codes
Yoric May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/12703.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Convert namespace class `Codes` into a string enum.
17 changes: 15 additions & 2 deletions synapse/api/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import logging
import typing
from enum import Enum
from http import HTTPStatus
from typing import Any, Dict, List, Optional, Union

Expand All @@ -30,7 +31,17 @@
logger = logging.getLogger(__name__)


class Codes:
class Codes(str, Enum):
"""
All known error codes, as an enum of strings.
"""

def __str__(self) -> str:
return self.value

def __repr__(self) -> str:
return repr(self.value)
Copy link
Contributor

@babolivier babolivier May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What are these for? I can't see a difference in behaviour between this version and how it works on develop:

This branch:

>>> from synapse.api.errors import Codes
>>> str(Codes.MISSING_TOKEN)
'M_MISSING_TOKEN'
>>> repr(Codes.MISSING_TOKEN)
"'M_MISSING_TOKEN'"
>>>

develop:

>>> from synapse.api.errors import Codes
>>> str(Codes.MISSING_TOKEN)
'M_MISSING_TOKEN'
>>> repr(Codes.MISSING_TOKEN)
"'M_MISSING_TOKEN'"
>>>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's to allow backwards compatibility.

Without these, Codes.foo becomes a Codes, which won't covert to a str, so we break all the sites that call e.g. Codes.MISSING_TOKEN.

Copy link
Contributor

@babolivier babolivier May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Without these, Codes.foo becomes a Codes, which won't covert to a str

But with current develop I'm able to do str(Codes.MISSING_TOKEN) and it returns the same thing than it does with this change (I've just noticed I didn't paste my snippet correctly above, fixed now), so I'm not sure I understand what you're talking about. Can you show me an example of code that wouldn't work without this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, it doesn't seem necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure?

>>> from enum import Enum
>>> class Codes(str, Enum):
...     MISSING_TOKEN = "M_MISSING_TOKEN"
...
>>> f"{Codes.MISSING_TOKEN}"
'M_MISSING_TOKEN'
>>> "%s" % (Codes.MISSING_TOKEN,)
'Codes.MISSING_TOKEN'
>>> str(Codes.MISSING_TOKEN)
'Codes.MISSING_TOKEN'


UNRECOGNIZED = "M_UNRECOGNIZED"
UNAUTHORIZED = "M_UNAUTHORIZED"
FORBIDDEN = "M_FORBIDDEN"
Expand Down Expand Up @@ -265,7 +276,9 @@ class UnrecognizedRequestError(SynapseError):
"""An error indicating we don't understand the request you're trying to make"""

def __init__(
self, msg: str = "Unrecognized request", errcode: str = Codes.UNRECOGNIZED
self,
msg: str = "Unrecognized request",
errcode: str = Codes.UNRECOGNIZED,
):
super().__init__(400, msg, errcode)

Expand Down