Skip to content

add support for including private Slack channels in slack source #636

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

Merged
merged 12 commits into from
Aug 12, 2025

Conversation

molkazhani2001
Copy link
Contributor

enh: add support for including private Slack channels in source

  • Added include_private_channels parameter to slack_source
  • Updated Slack API call to include private channels and group DMs

@molkazhani2001 molkazhani2001 requested a review from djudjuu July 28, 2025 12:18
mypy.ini Outdated
@@ -1,5 +1,5 @@
[mypy]
python_version=3.8
python_version=3.9
Copy link
Contributor

Choose a reason for hiding this comment

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

is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI was failing because a file inside the dlt package (postgres.py) uses parenthesized context managers a syntax that’s only valid in Python 3.9 and above
packages/dlt/destinations/impl/postgres/postgres.py:86: error: Parenthesized context managers are only supported in Python 3.9 and greater [syntax]

@djudjuu djudjuu requested a review from VioletM August 4, 2025 14:29
VioletM
VioletM previously approved these changes Aug 4, 2025
Copy link
Contributor

@VioletM VioletM left a comment

Choose a reason for hiding this comment

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

Looks very good!
Added a couple of minor comments

)

assert expected_message_table_name in table_names
assert expected_replies_table_name in table_names
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this table be created if we have zero messages available?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, because in three months the old messages will go away..good point. i guess we don't really need the replies test here

Co-authored-by: Violetta Mishechkina <sansiositres@gmail.com>
djudjuu and others added 3 commits August 5, 2025 09:44
@djudjuu djudjuu requested a review from VioletM August 5, 2025 11:25
@molkazhani2001 molkazhani2001 merged commit 870446c into master Aug 12, 2025
15 checks passed
@anuunchin anuunchin deleted the enh/slack_include_private_channels branch August 12, 2025 09:31
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.

3 participants