Skip to content
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

128-feature/show-subject-in-slack-and-telegram #129

Merged
merged 6 commits into from
Dec 4, 2023

Conversation

louisevelayo
Copy link
Collaborator

Description

Fixes #128

Changes made

  • Add subject argument to send_message functions in both bot_slack.py and bot_telegram.py.

Testing

  • Live messages now send a node down message. This is so that our tests can verify that any changes we make do not affect the formatting of the messages on either platform.

Additional comments

  • by merging this PR, the format on telegram messages will also be corrected

@louisevelayo louisevelayo linked an issue Dec 1, 2023 that may be closed by this pull request
@mourginakis
Copy link
Contributor

mourginakis commented Dec 1, 2023

itertools.batched is in python 3.12.

batched was in more-itertools in python 3.11

we could just define batched at the top and then add a todo to delete once python >3.12 becomes standard on all systems and after we update our docker image python version.

@mourginakis
Copy link
Contributor

Here's how I would do it. I'll leave it up to you whether you want to do it this way or not. There is no right answer here.

long_string = "This is a very long string that I want to split into batches of 10 characters."


def batched(iterable, n):
    from itertools import islice
    """Batch data into lists of length *n*. The last batch may be shorter.

    >>> list(batched('ABCDEFG', 3))
    [('A', 'B', 'C'), ('D', 'E', 'F'), ('G',)]

    On Python 3.12 and above, this is an alias for :func:`itertools.batched`.
    """
    if n < 1:
        raise ValueError('n must be at least one')
    it = iter(iterable)
    while True:
        batch = tuple(islice(it, n))
        if not batch:
            break
        yield batch


parts1 = [long_string[i:i+10] for i in range(0, len(long_string), 10)]
print(parts1)
parts2 = ["".join(batch) for batch in batched(long_string, 10)]
print(parts2)

assert parts1 == parts2

Comment on lines 127 to 134
if channels:
err1 = self.slack_bot.send_messages(channels, message)
err1 = self.slack_bot.send_messages(channels, subject, message)
if preferences['notify_telegram'] == True:
if self.telegram_bot:
chats = telegram_chats.get(node_provider_id, [])
if chats:
err2 = self.telegram_bot.send_messages(chats, message)
err2 = self.telegram_bot.send_messages(chats, subject, message)
return None
Copy link
Contributor

@mourginakis mourginakis Dec 1, 2023

Choose a reason for hiding this comment

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

Let's keep the send_messages() api the same as before, and instead concatenate the subject+message here, before the function gets called.

Comment on lines 16 to 17
max_message_length = 4096
message_parts = textwrap.wrap(message, width=max_message_length)
message_parts = [dispatch[i:i + max_message_length] for i in range(0, len(dispatch), max_message_length)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend defining and using batched here, but i'll let you decide what you want to do. If you decide to keep it this way, please add a comment TODO: use itertools.batched here when python version is updated to >=3.12.

If you decide to define batched here, please add a comment TODO: delete this for itertools.batched when python version is updated to >=3.12.

Comment on lines +120 to 122
dispatch = f"{subject}\n\n{message}"
if preferences['notify_email'] == True:
recipients = email_recipients.get(node_provider_id, [])
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@mourginakis mourginakis merged commit c77b027 into main Dec 4, 2023
1 check passed
@mourginakis mourginakis deleted the 128-feature/show-subject-in-slack-and-telegram branch December 4, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature/show-subject-in-slack-and-telegram
2 participants