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.

Sign up for GitHub

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

Jump to bottom

enable conversation sessions by default #6934

Merged
merged 4 commits into from
Oct 6, 2020
Merged

enable conversation sessions by default #6934

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
967eefe
delete duplicate session config constants
wochinge Oct 6, 2020
3ee3572
change default session timeout from 0 to 60 minutes
wochinge Oct 6, 2020
d3e0019
Merge branch '2.0.x' into session-config-default
wochinge Oct 6, 2020
dd6cf18
Merge branch '2.0.x' into session-config-default
rasabot Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions changelog/6934.removal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[Conversation sessions](domain.mdx#session-configuration) are now enabled by default
if your [Domain](domain.mdx) does not contain a session configuration.
Previously a missing session configuration was treated as if conversation sessions
were disabled. You can explicitly disable conversation sessions using the following
snippet:

```yaml-rasa title="domain.yml"
session_config:
# A session expiration time of `0`
# disables conversation sessions
session_expiration_time: 0
```
12 changes: 12 additions & 0 deletions docs/docs/migration-guide.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ how you can migrate from one version to another.
component in your `policies` configuration (`config.yml`) you can replace it
with `TEDPolicy`. It accepts the same configuration parameters.

* [Conversation sessions](domain.mdx#session-configuration) are now enabled by default
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 title of the targeted section is currently Session Configuration. I'd vote for renaming this to Conversation Sessions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erohmensing ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mm, I think it makes the most sense to have the title of the section to reflect the high level keys in the domain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, then let's leave it as it is.

if your [Domain](domain.mdx) does not contain a session configuration. Previously a
missing session configuration was treated as if conversation sessions were disabled.
You can explicitly disable conversation sessions using the following snippet:

```yaml-rasa title="domain.yml"
session_config:
# A session expiration time of `0`
# disables conversation sessions
session_expiration_time: 0
```

### Training data files

You can convert existing NLU, Stories, and NLG (i.e. `responses.md`) training data
Expand Down
3 changes: 0 additions & 3 deletions rasa/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@
ENV_SANIC_WORKERS = "SANIC_WORKERS"
ENV_SANIC_BACKLOG = "SANIC_BACKLOG"

DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES = 0
DEFAULT_CARRY_OVER_SLOTS_TO_NEW_SESSION = True

ENV_GPU_CONFIG = "TF_GPU_MEMORY_ALLOC"
ENV_CPU_INTER_OP_CONFIG = "TF_INTER_OP_PARALLELISM_THREADS"
ENV_CPU_INTRA_OP_CONFIG = "TF_INTRA_OP_PARALLELISM_THREADS"
2 changes: 1 addition & 1 deletion rasa/shared/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

DOMAIN_SCHEMA_FILE = "utils/schemas/domain.yml"

DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES = 0
DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES = 60
DEFAULT_CARRY_OVER_SLOTS_TO_NEW_SESSION = True

DEFAULT_NLU_FALLBACK_INTENT_NAME = "nlu_fallback"
Expand Down
2 changes: 0 additions & 2 deletions rasa/shared/core/domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ class SessionConfig(NamedTuple):

@staticmethod
def default() -> "SessionConfig":
# TODO: 2.0, reconsider how to apply sessions to old projects
return SessionConfig(
rasa.shared.constants.DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES,
rasa.shared.constants.DEFAULT_CARRY_OVER_SLOTS_TO_NEW_SESSION,
Expand Down Expand Up @@ -185,7 +184,6 @@ def from_dict(cls, data: Dict) -> "Domain":
def _get_session_config(session_config: Dict) -> SessionConfig:
session_expiration_time_min = session_config.get(SESSION_EXPIRATION_TIME_KEY)

# TODO: 2.0 reconsider how to apply sessions to old projects and legacy trackers
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we've just decided to leave them as is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I changed the default from 0 to 60 which essentially enables them by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

i guess i assumed that this note was about "should we read all of the conversations from 2018 as one conversation even if we introduce sessions now? with regards to applying them to legacy trackers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, okay. Yes, these will have sessions enabled, too, now

if session_expiration_time_min is None:
session_expiration_time_min = (
rasa.shared.constants.DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES
Expand Down
2 changes: 1 addition & 1 deletion tests/shared/core/test_domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest

import rasa.shared.utils.io
from rasa.constants import DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES
from rasa.shared.constants import DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES
from rasa.core import training, utils
from rasa.core.featurizers.tracker_featurizers import MaxHistoryTrackerFeaturizer
from rasa.shared.core.slots import TextSlot
Expand Down