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

Follow-up bugfix to merging separate domain files with custom session config #11433

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Follow-up bugfix to merging separate domain files with custom session config #11433

merged 2 commits into from
Aug 11, 2022

Conversation

ancalita
Copy link
Member

@ancalita ancalita commented Aug 11, 2022 •
edited
Loading

Proposed changes:

  • Fixes a particular bug that emerges when at least two separate domain files are being merged, where one contains a custom session config and the other files contain no session config. The latter ends up overriding the custom config because it is loaded as a Domain with a default session config since it contains no session config specification and the checks in merge_domain_dicts did not protect against this case.
  • This is a follow-up to the comments here: Setting domain session_expiration_time has no effect #11061 (comment)

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)

@ancalita ancalita marked this pull request as ready for review August 11, 2022 16:23
@ancalita ancalita requested a review from a team as a code owner August 11, 2022 16:23
@ancalita ancalita requested review from losterloh and removed request for a team August 11, 2022 16:23
@github-actions
Copy link
Contributor

🚀 A preview of the docs have been deployed at the following URL: https://11433--rasahq-docs-rasa-v2.netlify.app/docs/rasa

losterloh
losterloh approved these changes Aug 11, 2022
View reviewed changes
Copy link
Contributor

@losterloh losterloh left a comment

Choose a reason for hiding this comment

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

🔥 🚀 💯 and all the approval emojis! Great work!

@ancalita ancalita merged commit 9268b74 into 3.2.x Aug 11, 2022
@ancalita ancalita deleted the bugfix-session-config-follow-up branch August 11, 2022 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@losterloh losterloh losterloh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ancalita @losterloh