fix(source_slack): use private channels config when reading slack threads #39416
Conversation
| The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
octavia-squidington-iii added area/connectors 
ctdio had a problem deploying to community-ci-auto 
GitHub Actions Failure 
ctdio force-pushed the source_slack_use_private_channels_config_in_threads branch from 6d40d1a to 3ba5371 Compare ctdio had a problem deploying to community-ci-auto GitHub Actions Failure 
ctdio force-pushed the source_slack_use_private_channels_config_in_threads branch from 3ba5371 to dbb74fa Compare ctdio had a problem deploying to community-ci-auto GitHub Actions Failure 
austinkelleher left a comment
austinkelleher left a comment There was a problem hiding this comment.
LGTM! Looking forward to seeing this fix in place.
marcosmarxm left a comment
marcosmarxm left a comment There was a problem hiding this comment.
@ctdio can you bump the connector version in the metadata.yml, project.toml and the doc changelog entry? After run poetry lock inside the connector folder to update the dependencies. After I'll trigger CI tests to validate your changes.
| Updated. Thanks @marcosmarxm! |
poetry.lock Outdated
There was a problem hiding this comment.
Not this file. You need to navigate inside the connector folder and run inside it.
cd airbyte-integrations/connectors/source-slack poetry lock Please revert this change and follow the steps above.
There was a problem hiding this comment.
Oh shoot, my bad. Fixing that now
There was a problem hiding this comment.
Fixed @marcosmarxm. Thanks for catching that
ctdio force-pushed the source_slack_use_private_channels_config_in_threads branch from 10b98ef to 8791f90 Compare 
| Running CI tests let's wait for the results @ctdio |
marcosmarxm left a comment
marcosmarxm left a comment There was a problem hiding this comment.
LGTM @ctdio only need an additional ✅ from the connector team as Slack is a certified connector.
natikgadzhi left a comment
natikgadzhi left a comment There was a problem hiding this comment.
<3 @ctdio thank you for the contribution!
There are things to share ;)
We're migrating our sources to be as low-code as possible. In Slack, we only have this last stream (threads) that is in Python, the rest are manifest as you have seen.
Here's the PR to migrate Slack Threads stream to yaml by @tolik0: #38618. This PR in turn depends on the CDK PR #38211 that's approved, and will likely be merged this week.
This Slack Threads → lowcode PR is a "breaking change", meaning users would have to re-sync their Threads stream when they upgrade to source Slack 2.0.0
With that in mind, we can do a few things here.
- We can merge this PR now, and then @tolik0 will add private channel threads support to the low-code one before releasing #38618.
- We can skip this PR and just ask @tolik0 to add privae channels support to his PR.
Since this looks clean, I'm not opposed to merging this now, so — LGTM from where I sit.
| @irvingpop, looky here! Getting closer. |
natikgadzhi temporarily deployed to community-ci-auto GitHub Actions Inactive natikgadzhi temporarily deployed to community-ci-auto GitHub Actions Inactive | Hey @natikgadzhi, thanks for taking a look at this! I would appreciate it if we could go with option 1 and release this before migrating to the low-code implementation. We are hoping this could land in production sooner rather than later since it's a pretty severe gap in functionality we've encountered that's affecting business operations for us (we're using Airbyte for a number of customer integrations). |
| Yes, happy to! Thank you, @marcosmarxm and @natikgadzhi, for helping me get this through! |
…711/airbyte into krishan711/source-gong-extensive * 'krishan711/source-gong-extensive' of github.com:krishan711/airbyte: source-mysql: upgrade debezium to 2.5.4 (airbytehq#39144) fix(source_slack): use private channels config when reading slack threads (airbytehq#39416) ✨Source Linkedin Pages: migrate to low-code (airbytehq#36744) feat(docs): update metric icon (airbytehq#39449) Source Datadog: Chaged last records to last record (airbytehq#37590) 🐛 Source Kafka: fix empty airbyte_data after airbyte version 0.50.4 for AvroFormat (airbytehq#32538)
…eads (#39416) Co-authored-by: Natik Gadzhi <natik@respawn.io>
…eads (#39416) Co-authored-by: Natik Gadzhi <natik@respawn.io>
6 participants
What
This fixes #39414. This PR allows for threads in private channels to be synced by using the existing
include_private_channelsconfig option. This option is used for consuming messages and channels from the declarative streams, but was not applied to the threads stream.How
This change passes in the
include_private_channelsconfiguration into theThreadconstructor, which passes the value along to theChannel. Ifinclude_private_channelsis true, thetypesparameter in thechannelsrequest will be set toprivate_channel,public_channel, allowing private channels to be used when collecting threads.Can this PR be safely reverted and rolled back?