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

Do not reload installed packages on file change #842

Merged
merged 1 commit into from Mar 28, 2024

Conversation

rickythefox
Copy link
Contributor

Closes #790 .

The issue was present when the venv was created in the same dir as the project using chainlit. On reload the code that was supposed to clear the modules related to the app from sys.modules also cleared chainlit itself which led to a reload and a new config object being initialized when config.py module was loaded. The socket.py module still had the reference to the old config module though, which resulted in reload failing. The lesson here is - module reloading in python is HARD and should really not be attempted. ;)

The fix checks where the modules are installed (venv or regular installation) and prevents their reload. This fixes all cases EXCEPT the hello.py in the chainlit repo - as the chainlit code is in the same dir as the demo file it's imporsible to say what's what. A possible fix would be to move hello.py to a subdir.

@willydouhard
Copy link
Collaborator

This looks awesome! Going to do a bit of manual testing before merging. Thanks!

@rickythefox
Copy link
Contributor Author

So did you get this to work? Any corner cases I need to take into account?

Copy link
Collaborator

@tpatel tpatel left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution ✨

I couldn't replicate the file watcher issue, so I can't be sure it's fixing all the issues mentioned, but the explanation makes sense and I see no bad things that could come form this!

@tpatel tpatel merged commit 4d81836 into Chainlit:main Mar 28, 2024
4 checks passed
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.

Investigate and fix file watcher issues
3 participants