Slack bridge: Update Slack bridge with Events API. #826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
from
d22ae40
to
3ca5796
Compare
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
4 times, most recently
from
8ad1ab0
to
10bcf4f
Compare
|
ERROR: Label "buddy review" does not exist and was thus not added to this pull request. |
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
3 times, most recently
from
679f982
to
bc44575
Compare
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Jun 26, 2024
The github action running the tests uses Python 3.8.18. When running ./tools/run-mypy using this Python version there will be linting errors related to importing the 'Salesforce' class. However, this is not a problem when running the mypy linter under Python 3.10. This commit ignores these linting error just so that the main PR zulip#826 can pass the github actions tests. I wouldn't recommend merging this commit to permanently fix this issue. Instead, it might be better to update the github actions to use a more recent Python version.
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
from
add6643
to
e682ef6
Compare
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Jun 26, 2024
This is an optional commit that ignores linting errors in an unrelated file from zulip#826.
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Jun 28, 2024
This is an optional commit that ignores linting errors in an unrelated file from zulip#826.
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
from
42e472c
to
ded0ea7
Compare
rht
approved these changes
Jun 29, 2024
rht
reviewed
Jun 29, 2024
| It starts with "xoxp-...". | ||
|
|
||
| 4. In the `slack` section of the Zulip-Slack bridge configuration file, enter the bot name | ||
| (e.g "zulip_mirror"), token (e.g xoxp-...), and the channel ID (note: must be ID, not name). |
There was a problem hiding this comment.
Optional: IIRC xoxb- (not xoxp-) is the legacy token? We could document this, to explicitly say that it is legacy.
There was a problem hiding this comment.
Correct, the new token is xoxb. Updated the doc 👍
There was a problem hiding this comment.
I had this in mind, might as well say it: what if we add detection of the token prefix in the code, that gives a clear error message if user specifies an xoxp- token.
There was a problem hiding this comment.
hmm, yeah after reviewing the setup instructions again I feel like there is a lot of room for human error. Adding error messages like this would help reduce the probability.
This commit updates the Slack Bridge doc, primarily guiding the user to use our Slack Webhook integration and reformating the old instructions to a separate legacy section.
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Jun 29, 2024
This is an optional commit that ignores linting errors in an unrelated file from zulip#826.
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
from
eda7a33
to
d8e6f71
Compare
|
Update: Addressed review and dropped support for RTM API |
|
@sbansal1999 I think this one's also ready for mentor review, please do check it out! Thanks |
Slack Bridge now uses the Slack Webhook integration to get messages accross from Slack instead of the legacy RTM API based connection our Slack Bridge use. This commit adds a "--legacy" argument to the script, it acts as a toggle to run the RTM API based connection to get messages accross to Zulip. It is used to ensure backwards compitability for users who want to maintain any ongoing Slack mirror.
When using Slack Webhook integration to get messages from Slack to Zulip, we don't want to send back messages from the Slack integration bot. Fixes zulip#825.
This is an optional commit that upgrade the Python version for github workflows when running tests. This commit doesn't try to drop support for Python 3.8.
This is an optional commit that ignores linting errors in an unrelated file from zulip#826.
This is an optional commit that drops support running the Slack Bridge using the legacy RTM API. The reasoning for removing backwards compitability is because the user can just use the older version of pyton-zulip-api.
PieterCK
force-pushed
the
issue-slack-bridge-update-825
branch
from
d8e6f71
to
e0f20f6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we use the Slacks legacy RTM API as the "listener" from Slack to Zulip. This commit updates our Slack Bridge to use Webhook integration to get messages across from Slack to Zulip.
We use our Slack webhook integration for that part of the bridge, it is being updated to use the latest Event API at PR#30465. So, instead the changes in this PR are only the following:
Fixes #825.