uvicorn icon indicating copy to clipboard operation
uvicorn copied to clipboard

:bug: Fixed --reload-dir option not working as expected.

Open bybatkhuu opened this issue 2 years ago • 5 comments

--reload-dir specified, but default dir(cwd) still been watching

Not working as cli help explained:

txt

uvicorn --help:
--reload-dir PATH   Set reload directories explicitly, instead
                    of using the current working directory.

So, I tried to fix it by counting self.reload_dirs, and if it's not provided by arguments, then it will add the default current directory as the watching directory.

Previous discussion: https://github.com/encode/uvicorn/discussions/1833

And also: https://github.com/encode/uvicorn/issues/1326

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.

bybatkhuu avatar Sep 21 '23 13:09 bybatkhuu

This solution looks fine for me on a quick glance. I'll see if I can review this properly before giving my verdict. Also, Fix the tests please :)

iudeen avatar Sep 22 '23 20:09 iudeen

This issue can block the usage of the reload feature in some cases. I work on a CLI tool that should start the server from a host directory where a lot FS operations happen every few seconds. In my case, reload-dir targets the correct directory with the server source code, but those lines cause constant reload because Path.cwd() is added to the reload_dirs no matter what.

edit: This issue is also hard to detect because logs print directories it plans to watch before it adds Path.cwd() there.

INFO:     Will watch for changes in these directories: ['directories form --reload-dir']

imdoroshenko avatar Dec 06 '23 21:12 imdoroshenko

This is a bit suboptimal DX. Any help needed with this fix? I can fix the tests if @bybatkhuu doesn't have time. LMK.

akefirad avatar Jan 02 '24 09:01 akefirad

Sorry, I don't have time to understand tests and fix for this case. @akefirad, of course. Thank you~

bybatkhuu avatar Jan 02 '24 09:01 bybatkhuu