Skip to content

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

Closed
adhami3310 wants to merge 3 commits into
Kludex:masterfrom
adhami3310:relax-reload
Closed

adhami3310 wants to merge 3 commits into
Kludex:masterfrom
adhami3310:relax-reload

Conversation

@adhami3310

@adhami3310 adhami3310 commented Feb 13, 2025 •

Copy link
Copy Markdown

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.

@stinovlas stinovlas left a comment

Copy link
Copy Markdown
Contributor

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 :-).

self.reload_dirs.append(directory)
if Path.cwd() not in self.reload_dirs:
self.reload_dirs.append(Path.cwd())
self.reload_dirs.append(directory)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

maybe just do self.reload_dirs = config.reload_dirs ?

@adhami3310 adhami3310 closed this Jul 29, 2025
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.

3 participants