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

feat: Added slack chat integration. #236

Merged
merged 47 commits into from
Apr 6, 2024
Merged

Conversation

osala-eng
Copy link
Contributor

Description

Slack chat bot integration

Implements #227

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactor and code improvement (non-breaking change which improves code quality and/or performance)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation
  • Tests
  • Other chores such as maintenance

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration by modifying the list below.

  • Test A
  • Test B
  • Test C

Test Configuration:

Please describe the test setup. List them below as bullet points.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • The commit message follows the convention of this project

@osala-eng osala-eng requested a review from a team as a code owner March 8, 2024 21:58
@osala-eng osala-eng linked an issue Mar 8, 2024 that may be closed by this pull request
@osala-eng
Copy link
Contributor Author

osala-eng commented Mar 8, 2024

@janaka I've setup slack chat bot. you can review this and tell me what you think.

I've also added the sample chat bot to the Docq.AI's slack work space you can try that out aswell.

image

@janaka
Copy link
Contributor

janaka commented Mar 8, 2024

Great. I'll take a look.

@janaka
Copy link
Contributor

janaka commented Mar 9, 2024

How do I do RAG? Or you waiting for the updated Web API live?

@osala-eng
Copy link
Contributor Author

How do I do RAG? Or you waiting for the updated Web API live?

This just implements chat comletion as described in #227, once we get this approved i'll use it to implement Retrival in #228

@janaka
Copy link
Contributor

janaka commented Mar 9, 2024

great, I'll take a look at the code this weekend.

I shipped the new Web API btw.

Copy link
Contributor

@janaka janaka left a comment

Choose a reason for hiding this comment

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

Looks like the whole integration is webhooks based?

This code should be part of the API I think. It also should be designed as a general webhook handler ideally.

@janaka
Copy link
Contributor

janaka commented Mar 12, 2024

@osala-eng you see my comment about taking a look at Nango.dev as an alternative approach? I created an account in their SaaS and sent you an invite. If we go down this path will have to self-host though. This approach would definitely allow us to have a more general API our end.

@osala-eng
Copy link
Contributor Author

osala-eng commented Mar 12, 2024 via email

@osala-eng osala-eng marked this pull request as draft March 15, 2024 01:12
Copy link
Contributor

@janaka janaka left a comment

Choose a reason for hiding this comment

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

See comments. Please respond inline.

I've not paid attention to the integrations setting / install stuff because it seems you are still working on the changes we discussed, moving those to the Admin Section.

source/docq/integrations/slack.py Outdated Show resolved Hide resolved
source/docq/setup.py Outdated Show resolved Hide resolved
source/docq/support/auth_utils.py Outdated Show resolved Hide resolved
source/docq/support/auth_utils.py Outdated Show resolved Hide resolved
web/api/integration/slack/slack_application.py Outdated Show resolved Hide resolved
web/api/integration/slack/oauth_flow.py Outdated Show resolved Hide resolved
web/api/integration/slack/chat_handler.py Show resolved Hide resolved
@osala-eng osala-eng force-pushed the feat/slack-chat-integration-1 branch from 517cc90 to 5211574 Compare March 20, 2024 16:14
@osala-eng osala-eng marked this pull request as ready for review March 22, 2024 23:02
@osala-eng
Copy link
Contributor Author

@janaka I've updated the changes as requested and implemented end to end slack chat with docq bot using RAG.

image

@janaka
Copy link
Contributor

janaka commented Mar 22, 2024

Great. Will take a look tomorrow.

@janaka
Copy link
Contributor

janaka commented Mar 23, 2024

@osala-eng sorry, I merged the Llama Index update last night. Can you rebase and check all works. Mainly with the bits that bit the Web API

@osala-eng
Copy link
Contributor Author

@osala-eng sorry, I merged the Llama Index update last night. Can you rebase and check all works. Mainly with the bits that bit the Web API

Okay, let me rebase and test this.

@janaka
Copy link
Contributor

janaka commented Mar 24, 2024 via email

@osala-eng
Copy link
Contributor Author

@osala-eng I pushed a bunch of refactors. Any chance you can do a little quick test.

Okay let me do a quick test and get back to you.

Copy link
Contributor Author

@osala-eng osala-eng left a comment

Choose a reason for hiding this comment

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

This dosent work fully as some of the functionality has been disabled.

source/docq/integrations/slack/slack_oauth_flow.py Outdated Show resolved Hide resolved
@osala-eng
Copy link
Contributor Author

@janaka This is now ready.

@janaka
Copy link
Contributor

janaka commented Apr 5, 2024

@osala-eng did you write down anything / ref a link on how the values for the following were generated?

SLACK_CLIENT_ID
SLACK_CLIENT_SECRET
SLACK_SIGNING_SECRET

@janaka
Copy link
Contributor

janaka commented Apr 6, 2024

@osala-eng did you write down anything / ref a link on how the values for the following were generated?

SLACK_CLIENT_ID SLACK_CLIENT_SECRET SLACK_SIGNING_SECRET

@osala-eng I just remembered about the other repo with the manifest file which answer my original question sufficiently.

New related question about the env var names. You went with the SDK built-in names rather than our usual pattern of prefixing with "DOCQ_". Was that intentional for reasons like it's the only way to use the SDK? I wanted to check before refactoring to save myself some pain :)

@osala-eng
Copy link
Contributor Author

osala-eng commented Apr 6, 2024

@osala-eng did you write down anything / ref a link on how the values for the following were generated?
SLACK_CLIENT_ID SLACK_CLIENT_SECRET SLACK_SIGNING_SECRET

@osala-eng I just remembered about the other repo with the manifest file which answer my original question sufficiently.

New related question about the env var names. You went with the SDK built-in names rather than our usual pattern of prefixing with "DOCQ_". Was that intentional for reasons like it's the only way to use the SDK? I wanted to check before refactoring to save myself some pain :)

Yes, you can customize the env var names, then pass them explicitly to both App and SlackOauthFlow classes when instantiating them in the slack_application.py module. By default, slack_sdk looks for the names provided in their documentation.

@janaka janaka merged commit de24797 into main Apr 6, 2024
1 check passed
@janaka janaka deleted the feat/slack-chat-integration-1 branch April 6, 2024 19:47
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.

Slack Chat integration
2 participants