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

Add ignore_permission_denied option #224

Merged
merged 11 commits into from
Aug 24, 2023

Conversation

aminalaee
Copy link
Contributor

@aminalaee aminalaee commented Apr 11, 2023

Closes #147

@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Merging #224 (2f96cfa) into main (ea789d3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #224   +/-   ##
=======================================
  Coverage   99.50%   99.51%           
=======================================
  Files           6        6           
  Lines         403      411    +8     
  Branches       82       83    +1     
=======================================
+ Hits          401      409    +8     
  Misses          1        1           
  Partials        1        1           
Files Changed Coverage Δ
watchfiles/run.py 100.00% <ø> (ø)
watchfiles/cli.py 100.00% <100.00%> (ø)
watchfiles/main.py 98.19% <100.00%> (+0.12%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea789d3...2f96cfa. Read the comment docs.

@samuelcolvin
Copy link
Owner

Looks like a great start.

The parameter should be called ignore_permission_denied, default false.

And it should cause only permission denied errors to be ignored.

@aminalaee aminalaee force-pushed the support-ignoring-errors branch from 3953c73 to d87625d Compare April 12, 2023 14:38
@aminalaee aminalaee force-pushed the support-ignoring-errors branch from 857911d to 4659abb Compare April 12, 2023 14:49
@aminalaee
Copy link
Contributor Author

Not sure why, but doing watchfiles --ignore-permission-denied / still fails with error locally, but the tests are passing.

@aminalaee aminalaee changed the title Support ignoring errors Add ignore_permission_denied option Apr 12, 2023
@samuelcolvin
Copy link
Owner

I'll review properly soon, but would be good if we respected an env variable so it could be set when using watchfiles with uvicorn etc. - that's much more common than the command line usage.

Copy link
Owner

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

and the env var I mentioned.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@aminalaee aminalaee force-pushed the support-ignoring-errors branch from 06930c6 to 2b26204 Compare May 21, 2023 08:11
@aminalaee
Copy link
Contributor Author

aminalaee commented May 21, 2023

Thanks for the review

watchfiles/main.py Outdated Show resolved Hide resolved
@samuelcolvin samuelcolvin merged commit ea31274 into samuelcolvin:main Aug 24, 2023
@samuelcolvin
Copy link
Owner

thanks so much.

@aminalaee aminalaee deleted the support-ignoring-errors branch August 31, 2023 16:28
@mshytikov
Copy link

Hi here, thank you for adding the --ignore-permission-denied

I just want to flag that I observed strange behaviour with it (Linux). Which might help other people to save some time.

It stops watching directories if permission-denied occurs on the first directory in the workdir.

Example you have a directory a, b, c:

  • if a has permission-denied then workdir is watched but ab and c are not watched anymore - which is unexpected. I would expect b and c to still be watched. 
  • if b has permission-denied then workdir, a, c  are watched and b is not - which is expected.
Directory setup
# LC_COLLATE="C.UTF-8"
# directories setup
mkdir tmp && cd tmp
mkdir a b c
touch a/test.txt b/test.txt c/test.txt
Example 1 (unexpected behaviour)
# set permissions on 'a' directory and make it unaccessible.
sudo chown -R root a b c  && sudo chmod -R go+rwx a b c 
sudo chmod go-rwx a 

# start watching files and send it to background 
watchfiles 'echo change' --ignore-permission-denied &

# change file in workdir and observe the change event 
echo '!' >> test.txt

# change files in 'b' and 'c', no change events observed.
echo '!' >> b/test.txt
echo '!' >> c/test.txt 

# stop watchfiles 
pkill watchfiles
Example 2 (expected behaviour)
# set permissions and make 'a' accessible and 'b' not. 
# change permissions on 'a' directory
sudo chown -R root a b c  && sudo chmod -R go+rwx a b c 
sudo chmod go-rwx b 

# start watching files and send it to background 
watchfiles 'echo change' --ignore-permission-denied &

# change in files in workdir, 'a' and 'c'  and observe all change event 
echo '!' >> test.txt
echo '!' >> a/test.txt
echo '!' >> c/test.txt 

# stop watchfiles 
pkill watchfiles

In uvicorn for example here is recommended to use env var (WATCHFILES_IGNORE_PERMISSION_DENIED) but it does not help because of above.
For my case the only way to avoid the issue while using uvicorn, is to set the --reload-dir, but this encode/uvicorn#2107 is not merged yet.

 

josephw added a commit to josephw/fava that referenced this pull request Mar 30, 2024
Bump the minimum watchfiles version to ensure that ignore_permission_denied,
added in samuelcolvin/watchfiles#224 and released
in https://github.com/samuelcolvin/watchfiles/releases/tag/v0.20.0
is present.
josephw added a commit to josephw/fava that referenced this pull request Mar 30, 2024
Bump the minimum watchfiles version to ensure that ignore_permission_denied,
added in samuelcolvin/watchfiles#224 and released
in https://github.com/samuelcolvin/watchfiles/releases/tag/v0.20.0
is present.
yagebu added a commit to beancount/fava that referenced this pull request Mar 31, 2024
Continue to watch files are replaced, such as by saving in vi,
by watching the parent directory instead.

Add a test, that fails under the previous version, for a modification
to a file after it is deleted and recreated.

Bump the minimum watchfiles version to ensure that ignore_permission_denied,
added in samuelcolvin/watchfiles#224 and released
in https://github.com/samuelcolvin/watchfiles/releases/tag/v0.20.0
is present.

Co-authored-by: Jakob Schnitzer <[email protected]>
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.

Permission denied despite using ignore_paths
3 participants