conversation_granularity for slack - per user | per user per channel | per user per thread #10086

srinivasupadhya · 2021-11-05T06:48:28Z

allow conversation_id to be per user | per user per channel | per user per thread

Proposed changes:

Allow multiple active conversations for a user on slack with bot

Status (please check what you already did):

added some tests for the functionality
updated the documentation
updated the changelog (please check changelog for instructions)
reformat files using black (please check Readme for instructions)

…r per thread

CLAassistant · 2021-11-05T06:48:34Z

All committers have signed the CLA.

ka-bu · 2021-11-08T09:38:16Z

Thanks for raising the issue and implementing the fix 🚀

What will happen next:

I'll enable the tests and will get some feedback from more people cause it looks to me like the conversation_id that you constructed might be a more reasonable default value for the sender_id than the current choice
we'll address the test issues / feedback / review items
we'll need to update the rasa docs (https://rasa.com/docs/rasa/connectors/slack/) and add a changelog entry

And then we can merge this into the new rasa 3.0.0 :)

rasa/core/channels/slack.py

* Handle syslog writing from the command line * add an arg in the command line to use syslog * respect standards * line was too long * new arg for command line * add sanic logs to syslog * del useless statement * quick fix * too many blank line * Use default communication with syslog * possibility to use a syslog server * fix indent * Fix for constants access * fix typo Co-authored-by: Joe Juzl <[email protected]> * fix typo * fix for constant definition in bad file * Black linter compliant * Function add_server_arguments had 26 lines of code (exceeds 25 allowed). * fix for bad merge * for lint compliance * add required param * adapt test for log enhancement Co-authored-by: Joe Juzl <[email protected]>

…nges

* Markers telemetry * Everything without tests * Specified events.json * Test added * Changelog entry * Naming fixes * Fix lint, fix CLI bug No way to actually change config path, now fixed with nargs * Add markers parsed telemetry * Document telemetry functions * always add ANY_MARKER; add test * Add complexity telemetry * Skip root marker to avoid double reporting total marker count Co-authored-by: Matthew Summers <[email protected]> Co-authored-by: ka-bu <[email protected]>

…adhya/rasa into slack_conversation_id

srinivasupadhya · 2021-11-09T04:15:14Z

@ka-bu added docs for the config. Changing default would be backward incompatible (new rasa with old conversations in tracker store) & hence haven't changed this.

Also, is there anyway we can backport this fix to 2.8.x release?

ka-bu · 2021-11-10T09:53:16Z

Thanks for the quick reply and addressing the feedback!

@ka-bu added docs for the config. Changing default would be backward incompatible (new rasa with old conversations in tracker store) & hence haven't changed this.

Great that you thought of this :) . I've been trying to get some feedback in the meantime on whether it is ok to make that breaking change for 3.0. For now, there is no feedback so your non-breaking version is what we proceed with, it seems.

Also, is there anyway we can backport this fix to 2.8.x release?

If you like, you can open another PR and apply the same changes there to get this merged into 2.8.x. as well (by choosing that version as a target for your merge then). Otherwise, we can open an issue and someone from our team will pick that up.

ka-bu · 2021-11-10T09:55:25Z

I like the new name for that setting and the names for the options 👍 -- Just one tiny linting issue so far from the checks.

…adhya/rasa into slack_conversation_id

srinivasupadhya · 2021-11-10T10:47:26Z

@ka-bu addressed the formatting issue. thanks for the inputs. Raised #10146 for 2.8.x

tayfun · 2021-11-10T10:58:16Z

@ka-bu addressed the formatting issue. thanks for the inputs, will raise a separate PR for 2.8.x.

Thanks for your contributions! Once this is merged you can cherry-pick your commit and create a new PR for 2.8.x and we should be quick to get that one in.

By the way, you can use pre-commit hooks to run checks locally or just use black manually to check formatting issues on your own machine.

ka-bu · 2021-11-10T11:50:54Z

Reminder: We also have to add a changelog entry (i.e. a <pr number>.improvement file with a description under the changelog folder)

srinivasupadhya · 2021-11-10T13:47:30Z

@ka-bu @tayfun everything seems to be green now. let me know if there is anymore work here.

ka-bu

Some tiny suggestions. Otherwise looks good to me 🚀

changelog/10086.misc.md

docs/docs/connectors/slack.mdx

rasa/core/channels/slack.py

ka-bu

Feel free to squash merge when ready 🚀

srinivasupadhya · 2021-11-10T15:38:22Z

@ka-bu seems like coverage report upload job failed. Not sure how to proceed. Also, #10146 is ready as well.

…markers_bugfix * 'markers_bugfix' of https://github.com/RasaHQ/rasa: Fix model dir (#10164) add changelog entry about tf2.6 update (#10153) conversation_granularity for slack - per user | per user per channel | per user per thread (#10086) add changelog fragment for docs changes in 3.0 fix syntax in training data example in the docs

srinivasupadhya added 2 commits November 5, 2021 12:12

allow conversation_id to be per user | per user per channel | per use…

94dc44e

…r per thread

make formatter

bed5ea5

srinivasupadhya requested a review from a team as a code owner November 5, 2021 06:48

srinivasupadhya requested review from ka-bu and removed request for a team November 5, 2021 06:48

fix typo

36134b0

srinivasupadhya mentioned this pull request Nov 5, 2021

Slack channel - start new conversation per thread #9918

Closed

srinivasupadhya added 3 commits November 5, 2021 15:32

Merge branch 'main' into slack_conversation_id

34954e6

Merge branch 'main' into slack_conversation_id

7b053ca

Merge branch 'main' into slack_conversation_id

5a8b91b

tayfun reviewed Nov 8, 2021

View reviewed changes

rasa/core/channels/slack.py Outdated Show resolved Hide resolved

srinivasupadhya and others added 17 commits November 8, 2021 16:59

fix review comment

8861c8e

Update sanic to make Rasa compatible with the latest rasa-sdk cha…

58cb53b

…nges

Reverting back the recent <conversation_id>

4ffb4f1

Update for list_routes

92224e9

Updated dependencies + test_server.py indentation

ae363f4

Changelog entry

9b8a8c0

Updated list_routes

e331d88

TF updates

b9ad076

fix pytest sanic

7b9c1c0

fix test_hangouts_extract_functions

3dcb737

fix route tests

f6f3918

black format

afe3e60

fix mypy issues

c4e5f50

temporarily disable no-untyped-call

87b6d5c

increase timeout

0914b05

srinivasupadhya added 4 commits November 8, 2021 17:49

Merge branch 'slack_conversation_id' of https://github.com/srinivasup…

0aee987

…adhya/rasa into slack_conversation_id

docs

e965d86

handle empty channel / thread

140af34

add test

a80fd1c

srinivasupadhya changed the title ~~allow conversation_id to be per user | per user per channel | per user per thread~~ conversation_granularity for slack - per user | per user per channel | per user per thread Nov 9, 2021

Merge branch 'main' into slack_conversation_id

ab479e4

srinivasupadhya requested a review from tayfun November 10, 2021 02:11

srinivasupadhya added 2 commits November 10, 2021 16:14

formatting

fdeb4d1

Merge branch 'slack_conversation_id' of https://github.com/srinivasup…

d4bd592

…adhya/rasa into slack_conversation_id

make get_conversation_id private

412896a

srinivasupadhya added 2 commits November 10, 2021 17:30

fix make types

2cdefb7

update changelog

aa8c1a7

ka-bu reviewed Nov 10, 2021

View reviewed changes

changelog/10086.misc.md Outdated Show resolved Hide resolved

docs/docs/connectors/slack.mdx Show resolved Hide resolved

rasa/core/channels/slack.py Show resolved Hide resolved

update changelog

8e006b8

ka-bu approved these changes Nov 10, 2021

View reviewed changes

tayfun approved these changes Nov 10, 2021

View reviewed changes

tayfun mentioned this pull request Nov 10, 2021

Rename sender_id in channel connectors to conversation_id #10158

Closed

Merge branch 'main' into slack_conversation_id

5a71d1c

Merge branch 'main' into slack_conversation_id

b5636a7

tayfun enabled auto-merge (squash) November 10, 2021 15:40

tayfun merged commit d04290f into RasaHQ:main Nov 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

conversation_granularity for slack - per user | per user per channel | per user per thread #10086

conversation_granularity for slack - per user | per user per channel | per user per thread #10086

srinivasupadhya commented Nov 5, 2021 •

edited

Loading

CLAassistant commented Nov 5, 2021 •

edited

Loading

ka-bu commented Nov 8, 2021

srinivasupadhya commented Nov 9, 2021 •

edited

Loading

ka-bu commented Nov 10, 2021

ka-bu commented Nov 10, 2021

srinivasupadhya commented Nov 10, 2021 •

edited

Loading

tayfun commented Nov 10, 2021

ka-bu commented Nov 10, 2021

srinivasupadhya commented Nov 10, 2021

ka-bu left a comment

ka-bu left a comment

srinivasupadhya commented Nov 10, 2021

conversation_granularity for slack - per user | per user per channel | per user per thread #10086

conversation_granularity for slack - per user | per user per channel | per user per thread #10086

Conversation

srinivasupadhya commented Nov 5, 2021 • edited Loading

CLAassistant commented Nov 5, 2021 • edited Loading

ka-bu commented Nov 8, 2021

srinivasupadhya commented Nov 9, 2021 • edited Loading

ka-bu commented Nov 10, 2021

ka-bu commented Nov 10, 2021

srinivasupadhya commented Nov 10, 2021 • edited Loading

tayfun commented Nov 10, 2021

ka-bu commented Nov 10, 2021

srinivasupadhya commented Nov 10, 2021

ka-bu left a comment

Choose a reason for hiding this comment

ka-bu left a comment

Choose a reason for hiding this comment

srinivasupadhya commented Nov 10, 2021

srinivasupadhya commented Nov 5, 2021 •

edited

Loading

CLAassistant commented Nov 5, 2021 •

edited

Loading

srinivasupadhya commented Nov 9, 2021 •

edited

Loading

srinivasupadhya commented Nov 10, 2021 •

edited

Loading