-
Notifications
You must be signed in to change notification settings - Fork 187
Add FileSystemMessagingIntegration and related tests #1904
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
Conversation
- Implemented FileSystemMessagingIntegration for sending messages to files. - Added unit tests for message sending, file creation, and handling of the create_if_missing flag. - Updated messaging integrations __init__.py to include the new integration.
👋 @MikaKerman |
try: | ||
logger.info("Writing alert message to file %s", file_path) | ||
with open(file_path, "a", encoding="utf-8") as fp: | ||
fp.write(body.json()) |
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.
add indent to the json, will make it much easier to debug
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.
Done.
if not os.path.exists(self.directory): | ||
if self._create_if_missing: | ||
logger.info( | ||
"Creating directory for FileSystemMessagingIntegration: %s", |
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.
why not direct formating? f"Creating directory for FileSystemMessagingIntegration: {self.directory}"
(relevant to all of the logs)
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.
Done.
BaseMessagingIntegration[str, EmptyMessageContext] | ||
): | ||
def __init__(self, directory: str, create_if_missing: bool = True) -> None: | ||
self.directory = os.path.abspath(directory) |
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.
I'd much prefer usage pathlib.Path
instead of os.path
, much cleaner and more modern
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.
Done.
…ngs and update tests to match new channel-directory JSON file behavior
Uh oh!
There was an error while loading. Please reload this page.