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.
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 |
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Jul 3, 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
d8e6f71
to
e0f20f6
Compare
This commit updates the Slack Bridge doc, primarily guiding the user to use our Slack Webhook integration.
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Oct 20, 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
e0f20f6
to
a394987
Compare
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Oct 20, 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
a394987
to
da246c4
Compare
Slack Bridge now uses the Slack Webhook integration to get messages accross from Slack instead of the legacy RTM API based we preivouslt use.
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. This prevents that by filtering out any messages from the Slack Webhook bots when sending messages from Zulip to Slack.. 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.
PieterCK
added a commit
to PieterCK/python-zulip-api
that referenced
this pull request
Oct 20, 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
da246c4
to
5f42745
Compare
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
5f42745
to
b4bcbf0
Compare
|
Heads up @PieterCK, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
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 Slack's legacy RTM API as the "listener" for messages from Slack to Zulip. This PR updates our Slack Bridge to use the Webhook integration to send messages from Slack to Zulip.
We now utilize the Slack Webhook integration for this part of the bridge, which is being updated to use the latest Event API in PR#30465. Here are the key changes in this PR :
salesforce.pythat was causing tests to fail.Fixes #825.