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

Setting domain session_expiration_time has no effect #11061

Closed
dgradwell-ams opened this issue Apr 5, 2022 · 29 comments · Fixed by #11324
Closed

Setting domain session_expiration_time has no effect #11061

dgradwell-ams opened this issue Apr 5, 2022 · 29 comments · Fixed by #11324
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework

Comments

@dgradwell-ams
Copy link

dgradwell-ams commented Apr 5, 2022

h3. Rasa Open Source version

3.1.0

h3. Rasa SDK version

No response

h3. Rasa X version

No response

h3. Python version

3.9

h3. What operating system are you using?

Linux

h3. What happened?

Since version 3.1.0, my preference for session_expiration_time (0) under session_config in the domain doesn't seem to be working anymore. The value is always being set to 60, the default.

I've confirmed that this is the actual value in three ways,

Inspecting the result of the API call to [get the domain| https://rasa.com/docs/rasa/pages/http-api#operation/getDomain] of the running bot

Extracting the trained model archive and inspecting the merged domain therein, and

Every 60 minutes of actual inactivity on my session, action_session_start is invoked

This happens whether my domain is contained in a single file or split into multiple files in a directory. I've tried deleting the trained models and the .rasa cache directory and retraining the model to no avail.

Thing is, we have overridden the action_session_start in our project, and it has considerable side effects in our app that are only meant to be run once for each conversation. We cannot call it again -- we don't want sessions to expire.

h3. Command / Request

No response

h3. Relevant log output

No response

h3. Reproduction repository

[https://github.com/dgradwell-ams/rasa-issue-11061| https://github.com/dgradwell-ams/rasa-issue-11061]

@dgradwell-ams dgradwell-ams added area:rasa-oss 🎡 Anything related to the open source Rasa framework type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Apr 5, 2022
@sara-tagger
Copy link
Collaborator

Thanks for raising this issue, @mvielkind will get back to you about it soon✨

Please also check out the docs and the forum in case your issue was raised there too 🤗

@rgstephens
Copy link
Contributor

Hi @dgradwell-ams, I can't duplicate this. Just tested session_expiration_time with a value of 3 and the session restarted when I was idle past the three minutes.

@dgradwell-ams
Copy link
Author

dgradwell-ams commented Apr 9, 2022

@rgstephens Try setting it to zero.

@dgradwell-ams dgradwell-ams changed the title Setting domain session_expiration_time has no effect Setting domain session_expiration_time to zero (0) has no effect Apr 11, 2022
@rgstephens
Copy link
Contributor

I tried zero and it worked for me. Started a session, waited more the 60 minutes, entered an utterance and session and slots were still set, no action_session_start.

I wonder if this could be channel specific. What channel are you using? I did my testing with rasa shell.

@dgradwell-ams
Copy link
Author

@rgstephens Hey, you might be on to something. We are using custom connectors, but perhaps I missed something when updating.

@dgradwell-ams
Copy link
Author

dgradwell-ams commented Apr 22, 2022

@rgstephens OK, so upon further investigation, we cannot see anything in our custom connector that would indicate how or why the domain session would not be properly setup. It's nearly identical to the example here: https://rasa.com/docs/rasa/connectors/custom-connectors

@dgradwell-ams
Copy link
Author

I'll try to setup a reproduction repo.

@dgradwell-ams
Copy link
Author

@rgstephens Here's a repo that reproduces the issue https://github.com/dgradwell-ams/rasa-issue-11061

I've set the expiration time to 1 minute in the configuration to demonstrate easily. See the README.

@dgradwell-ams dgradwell-ams changed the title Setting domain session_expiration_time to zero (0) has no effect Setting domain session_expiration_time has no effect Apr 23, 2022
@benos
Copy link

benos commented May 6, 2022

I also have the issue. The entire session_config seems to be ignored. I doubt that it is related to the connector since it seems to get baked into the model at training time.

Here is my config:

image

And here is the metadata.json of the trained model:

image

@benos
Copy link

benos commented May 6, 2022

@benos
Copy link

benos commented May 6, 2022

Note that if I hack the metadata.json in the trained model then things behave as expected.

@dgradwell-ams
Copy link
Author

The way that I worked around it was to patch the defaults in the core constants in my docker image to the values I need. This confirms that the defaults are being used.

@benos
Copy link

benos commented May 6, 2022

Thanks @dgradwell-ams That indeed sounds better than changing the metadata each time ...

Is this something you can do via the Dockerfile / docker-compose or do you have to modify constants.py and build your own image?

(and if the former, would you be able to share how you did it?)

@dgradwell-ams
Copy link
Author

I'm building my own image in a complicated process that makes it cross-platform between linux/amd64 and linux/arm64 so that it can be loaded on Apple Silicon for local development.....

In the image, I create a miniconda environment and install everything... then I just patch the constants.py using sed to change the one thing I need to change (session_expiration_time) to zero...

Relevant portion of Dockerfile for after rasa is installed:

# Patch the DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES in rasa's constants
# to workaround an bug in the domain merging code.
# Related github issue: https://github.com/RasaHQ/rasa/issues/11061
RUN sed -ie 's/DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES = 60/DEFAULT_SESSION_EXPIRATION_TIME_IN_MINUTES = 0/g' "${CONDA_ENV}/lib/python${PYTHON_VERSION}/site-packages/rasa/shared/constants.py"

@benos
Copy link

benos commented May 6, 2022

Thanks so much for the details @dgradwell-ams. As it happens I'm also dealing with local dev on the M1, but have opted to do that outside of Docker. So I think I'll stick with hacking the models after training for now.

Hopefully the team at Rasa can fix the issue in the next release. It's quite a nasty bug.

@dgradwell-ams
Copy link
Author

I would recommend using docker, I got better training times without Metal.

@benos
Copy link

benos commented May 11, 2022

You're probably right, but the overhead of figuring out how to create a Docker image that will work on the M1 is a bit much for me now. You can actually comment out tensorflow-metal in the requirements to solve the training times issues outside of Docker.

@benos
Copy link

benos commented May 31, 2022

@wochinge @erohmensing

Sorry to @ you in this way, but is the rasa team aware of this issue?

It is a nasty bug that will unexpectedly bite anyone relying on the session config. I also imagine it isn't a huge deal to fix

Thanks!

@ancalita
Copy link
Member

ancalita commented Jul 6, 2022

@dgradwell-ams Thanks for submitting a repo for reproducing the issue!
Confirming I managed to reproduce with Python 3.9 (albeit on a Mac machine with Intel chip) using your README instructions. I was expecting to see this debugging log message after 1 minute of waiting and retrying the curl command, however this is not the case.

Possibly worth investigating into whether Domain.session_config actually gets the configured values during merging of multiple domain files, or if it gets overwritten with the default values.

Do you experience the same issue when using a single domain file?

cc'ing our PM @chandrikas

@rgstephens
Copy link
Contributor

David is no longer working on the bot.

@benos
Copy link

benos commented Jul 15, 2022

Hi @ancalita

A bit late to the party, but I tested with a fresh rasa init (works fine) and then splitting a single response into a separate file (config gets ignored in the trained model), so I think you've nailed it. I've attached the repo of the latter in case it is of any use.

multiple_domain_file.tar.gz

Many thanks for looking into this!

Ben

@benos
Copy link

benos commented Aug 4, 2022

Hi @ancalita

From the changelog of 3.2.4 I understand the problem should be fixed with that release. However I find that not to be the case; if I train with a split domain file and then inspect the metadata.json of the model it still uses the default value.

Thanks,

Ben

@ancalita
Copy link
Member

ancalita commented Aug 8, 2022

@benos I went back and tested the bugfix release with the provided repo here. On all counts, the bug was fixed:

  • I tested with rasa run --enable-api --debug and test_session.sh as laid out in the reproduction steps in the Readme. As expected, after >1 min, I could see this in the logs:
2022-08-08 12:10:42 DEBUG    rasa.core.processor  - The latest session for conversation ID '78e07430-1c30-415a-8e7c-f9d84a16faf7' has expired.
2022-08-08 12:10:42 DEBUG    rasa.core.processor  - Starting a new session for conversation ID '78e07430-1c30-415a-8e7c-f9d84a16faf7'.
  • I then inspected the components/domain_provider/domain.yml: the session_expiration_time was still set as 1.

Screenshot 2022-08-08 at 12 18 00

  • I also inspected metadata.json, and the session_expiration_time was set as 1 there too.

Screenshot 2022-08-08 at 12 17 44

Do you have a repo example that I can test your scenario with?

@benos
Copy link

benos commented Aug 10, 2022

Hi @ancalita ,

I've attached a demo of the issue, which is a minor modification of the "rasa init" output.

rasa_session_bug.tar.gz

  • If you inspect the model you can see that the metadata.json and the domain provider both use default values. You can also see that it was trained with 3.2.4
  • If you delete data/domain_copy.yml and retrain, then the model uses the correct value

Best,

Ben

@ancalita
Copy link
Member

ancalita commented Aug 10, 2022

@benos What is the initial train command you use and how do you pass the two separate domain files to the training command? The expected way is to have all domain files in the same subdirectory. I would advise to create a separate folder for domain files rather than using data/

@benos
Copy link

benos commented Aug 10, 2022

HI @ancalita

  • rasa train
  • I don't do anything particular to pass the files. It just works

Note that the only thing that the domain file under 'data' contains are responses (so essentially a responses.yml).

To give some background, the way I've been working is to have the responses for a particular story contained within that story file, making it much easier to maintain.

From your response I assume that this is the wrong way and I should move them to one or more responses.yml files within the same directory as the domain. But I find that a real shame since it actually works great to combine the keys, except for the issue of the session config getting nixed.

Thanks!

Ben

@benos
Copy link

benos commented Aug 10, 2022

@ancalita To expand on that, if it is the case that one shouldn't mix the responses key with stories and rules then it probably shouldn't work and / or raise an error (though the way it is now is a feature not a bug IMO. Except for the session issue :-)

@ancalita
Copy link
Member

@benos A fix for your reported issue was released in rasa 3.2.6.

@benos
Copy link

benos commented Aug 14, 2022

@ancalita That is awesome news. Many thanks!

@exalate-issue-sync exalate-issue-sync bot removed type:bug 🐛 Inconsistencies or issues which will cause an issue or problem for users or implementors. triaged labels Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants