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

relax reload_dirs to allow files, and stop adding cwd when reload_dirs is set #2583

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adhami3310
Copy link

@adhami3310 adhami3310 commented Feb 13, 2025

Summary

Based on the discussion on #1833 the behavior currently done by a uvicorn is intuitive and most likely will lead to bugs by downstream users. It's also the case that watchfiles can watch individual files, as such it's a manufactured constraint by uvicorn is very arbitrary. Some other ASGIs just has reload_paths, that handles files just fine. I didn't change the variable name to not break backwards compatibility, but the former change can be seen as breaking, even if the existing behavior can be considered as a bug.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

I'm not familiar with the larger project, so I will be holding off on documentation and tests until it's verified that the project has interest in the change getting merged.

Copy link

@stinovlas stinovlas left a comment

Choose a reason for hiding this comment

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

Looks good to me! I bumped into the issue with --reload-dir not behaving like documentation states and this should fix it. Hopefully some maintainer will pick it up soon :-).

@adhami3310 adhami3310 mentioned this pull request Mar 21, 2025
3 tasks
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.

2 participants