-
Notifications
You must be signed in to change notification settings - Fork 0
format #4
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
base: main
Are you sure you want to change the base?
format #4
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| def pytest_itemcollected(item): | ||
| node = item.obj | ||
| if node.__doc__: | ||
| test_title_parts = item._nodeid.split("::") | ||
| test_title_parts = item._nodeid.split('::') | ||
| test_title_parts[-1] = node.__doc__.strip() | ||
| item._nodeid = "::".join(test_title_parts) | ||
| item._nodeid = '::'.join(test_title_parts) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,21 @@ | ||
| import contextlib | ||
| from typing import Dict, Literal, Optional | ||
|
|
||
| from system_notification.domain.exceptions.notification_error import TargetNotFound | ||
| from system_notification.domain.exceptions.notification_error import ( | ||
| TargetNotFound, | ||
| ) | ||
| from system_notification.domain.notifications.notification_target import ( | ||
| NotificationTarget, | ||
| ) | ||
| from system_notification.domain.protocols.notification_factory_protocol import ( | ||
| NotificationFactory, | ||
| ) | ||
| from system_notification.domain.protocols.notification_protocol import Notification | ||
| from system_notification.domain.protocols.notification_sender import NotificationSender | ||
| from system_notification.domain.protocols.notification_protocol import ( | ||
| Notification, | ||
| ) | ||
| from system_notification.domain.protocols.notification_sender import ( | ||
| NotificationSender, | ||
| ) | ||
|
|
||
|
|
||
| class NotificationFactoryCaller: | ||
|
|
@@ -19,18 +25,20 @@ def __init__(self) -> None: | |
| def add_factory(self, factory: NotificationFactory) -> None: | ||
| self._factories[factory.target_type] = factory | ||
|
|
||
| async def get_sender(self, target: NotificationTarget) -> Optional[NotificationSender]: | ||
| async def get_sender( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. async def get_sender(self, target: NotificationTarget) -> Optional[NotificationSender]:
...
raise TargetNotFound({
'is_sent': False,
'destin': {
'target_type': target.type,
'target': target.target,
},
'detail': 'unknown_handler',
})Unificar a lógica de tratamento de erros entre NotificationFactoryCaller e FactoryCaller, que atualmente tratam erros de forma inconsistente. Fale com o Kody mencionando @kody Essa sugestão foi útil? Reaja com 👍 ou 👎 para ajudar o Kody a aprender com essa interação. |
||
| self, target: NotificationTarget | ||
| ) -> Optional[NotificationSender]: | ||
| with contextlib.suppress(KeyError): | ||
| factory = self._factories[target.type] | ||
| return factory.make_sender() | ||
| raise TargetNotFound( | ||
| { | ||
| "is_sent": False, | ||
| "destin": { | ||
| "target_type": target.type, | ||
| "target": target.target, | ||
| 'is_sent': False, | ||
| 'destin': { | ||
| 'target_type': target.type, | ||
| 'target': target.target, | ||
| }, | ||
| "detail": "unknown_handler", | ||
| 'detail': 'unknown_handler', | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,21 +7,21 @@ | |
| NotificationTarget, | ||
| ) | ||
|
|
||
| T = TypeVar("T", covariant=True) | ||
| T = TypeVar('T', covariant=True) | ||
|
|
||
|
|
||
| @dataclass | ||
| class BaseNotification: | ||
| title: str | ||
| content: str | ||
| priority: Literal[0, 1, 2, 3] = 0 | ||
| icon: str = "" | ||
| icon: str = '' | ||
|
|
||
| def __post_init__(self) -> None: | ||
| self._is_sent: bool = False | ||
| self._vars: Dict[str, str] = {} | ||
| self._target: Optional[NotificationTarget] = None | ||
| self._status: str = "filled" | ||
| self._status: str = 'filled' | ||
| if not self.priority or type(self.priority) != int: | ||
| self.priority = 0 | ||
|
|
||
|
|
@@ -46,7 +46,7 @@ def status(self) -> str: | |
|
|
||
| @status.setter | ||
| def status(self, status: str) -> None: | ||
| if status.lower() in ("queued", "sent"): | ||
| if status.lower() in ('queued', 'sent'): | ||
| self._is_sent = True | ||
| self._status = status | ||
|
Comment on lines
48
to
51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @status.setter
def status(self, status: str) -> None:
if status.lower() in ('queued', 'sent'):
self._is_sent = True
else:
self._is_sent = False
self._status = statusProblemas de validação e tratamento de erros inconsistentes, onde exceções não são capturadas corretamente ou estados inconsistentes são mantidos. This issue appears in multiple locations:
Fale com o Kody mencionando @kody Essa sugestão foi útil? Reaja com 👍 ou 👎 para ajudar o Kody a aprender com essa interação. |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,31 @@ | ||
| from typing import Literal | ||
|
|
||
| from system_notification.domain.notifications.base_notification import BaseNotification | ||
| from system_notification.domain.protocols.notification_protocol import Notification | ||
| from system_notification.domain.protocols.notification_sender import NotificationSender | ||
| from system_notification.domain.notifications.base_notification import ( | ||
| BaseNotification, | ||
| ) | ||
| from system_notification.domain.protocols.notification_protocol import ( | ||
| Notification, | ||
| ) | ||
| from system_notification.domain.protocols.notification_sender import ( | ||
| NotificationSender, | ||
| ) | ||
| from system_notification.infra.notification_handlers.slack_notification_handler import ( | ||
| SlackNotificationHandler, | ||
| ) | ||
|
|
||
|
|
||
| class SlackNotificationFactory: | ||
| target_type = "slack_channel" | ||
| target_type = 'slack_channel' | ||
|
|
||
| def __init__(self, slack_token: str) -> None: | ||
| self._slack_token = slack_token | ||
|
|
||
| def make_notificaton( | ||
| self, title: str, content: str, priority: Literal[0, 1, 2, 3] = 0 | ||
| ) -> Notification: | ||
| return BaseNotification(title=title, content=content, priority=priority) | ||
| return BaseNotification( | ||
| title=title, content=content, priority=priority | ||
| ) | ||
|
|
||
| def make_sender(self) -> NotificationSender: | ||
| return SlackNotificationHandler(token=self._slack_token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problema de estado compartilhado em ambientes concorrentes, onde uma única instância de classe é usada para múltiplos requests, causando condições de corrida e corrupção de estado.
This issue appears in multiple locations:
Remova o estado compartilhado da instância da classe e armazene o estado de autenticação no objeto 'request' ou em um contexto específico de cada request.
Fale com o Kody mencionando @kody
Essa sugestão foi útil? Reaja com 👍 ou 👎 para ajudar o Kody a aprender com essa interação.