Replace `watchgod` library with `watchfiles`

jitu5 commented 1 month ago

Description

Resolves https://github.com/kedro-org/kedro-viz/issues/1589 and https://github.com/kedro-org/kedro-viz/issues/2135

This pull request includes

Replacing the `watchgod` library with `watchfiles`.

watchfiles library updates its API with main method run_process and some other changes as well:

watcher_cls is removed and replaced by watch_filter which should be a simple callable, see filter docs
run_process method allow multiple paths to be watched, as result, the target argument to run_process & arun_process is now keyword-only

Please refer migration guild for more details.

Custom File Filter:

package/kedro_viz/autoreload_file_filter.py: Introduced AutoreloadFileFilter class to filter files based on allowed extensions and .gitignore patterns.
package/tests/test_autoreload_file_filter.py: Added tests for the new AutoreloadFileFilter class.

Development notes

Library Replacement and File Watching Mechanism Updates:

Replaced watchgod with watchfiles across various files (package/kedro_viz/launchers/cli/run.py, package/kedro_viz/launchers/jupyter.py, package/kedro_viz/server.py, package/tests/test_launchers/test_cli/test_run.py, package/tests/test_launchers/test_jupyter.py)
Add autoreload_file_filter.py class watch file filter package/tests/test_autoreload_file_filter.py

Dependency Updates:

Updated dependencies in requirements files to replace watchgod with watchfiles (package/features/steps/lower_requirements.txt, package/requirements.txt)

Checklist

[x] Read the contributing guidelines
[x] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
[ ] Updated the documentation to reflect the code changes
[x] Added new entries to the RELEASE.md file
[x] Added tests to cover my changes

ravi-kumar-pilla commented 1 month ago

Hi @jitu5 ,

This may not be related to watchgod -> watchfiles migration. But I am seeing more error logs when I edit files i.e.,

Start in autoreload mode - kedro viz -a
Start editing .py or any file that is being watched
Save the changes and below error (similar errors) appear before rebuilding the app (this was raised earlier as well)
Even without editing the file, if you just save the file, i.e., you are on .py file and do cmd+s -> raises error in the terminal

If possible, create a new ticket to have debounce for watching files and rebuilding the app or handle these errors someway for a cleaner watch experience. Since these errors have become frequent now, I would suggest we think of a solution before releasing this migration. Thank you

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/__init__.py", line 16, in <module>
    from . import context
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/context.py", line 6, in <module>
    from . import reduction
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/reduction.py", line 15, in <module>
    import pickle
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/pickle.py", line 43, in <module>
    from _pickle import PickleBuffer
KeyboardInterrupt

jitu5 commented 1 month ago

Hi @jitu5 ,

This may not be related to watchgod -> watchfiles migration. But I am seeing more error logs when I edit files i.e.,

Start in autoreload mode - kedro viz -a

Start editing .py or any file that is being watched

Save the changes and below error (similar errors) appear before rebuilding the app (this was raised earlier as well)

Even without editing the file, if you just save the file, i.e., you are on .py file and do cmd+s -> raises error in the terminal

If possible, create a new ticket to have debounce for watching files and rebuilding the app or handle these errors someway for a cleaner watch experience. Since these errors have become frequent now, I would suggest we think of a solution before releasing this migration. Thank you
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/__init__.py", line 16, in <module>
    from . import context
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/context.py", line 6, in <module>
    from . import reduction
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/multiprocessing/reduction.py", line 15, in <module>
    import pickle
  File "/Users/Ravi_Kumar_Pilla/opt/anaconda3/envs/kedro-viz-py39/lib/python3.9/pickle.py", line 43, in <module>
    from _pickle import PickleBuffer
KeyboardInterrupt

@ravi-kumar-pilla I created a new virtual environment and followed the above steps to test but I didn't get any errors in logs. Could you try after crating new virtual environment. Also about Debouncing, run_process has debounce/step parameter, which we can use to introduce debounce for watcher.

Huongg commented 4 weeks ago

Hey @jitu5 after testing your PR, i got a similar error as @ravi-kumar-pilla has too

jitu5 commented 3 weeks ago

Hey @jitu5 after testing your PR, i got a similar error as @ravi-kumar-pilla has too

@Huongg The errors you shared are not the same as Ravi got, though they may be related to multiprocessing. Could you please share steps to reproduce the same error? Also have you created a new virtual environment to test ?

Huongg commented 3 weeks ago

hey @jitu5 I tried a different environment and the error is clear now, it could have the previous environment. But I know have a different error but i might not be related to your changes numpy.core.multiarray failed to import

rashidakanchwala commented 3 weeks ago

hey @jitu5 I tried a different environment and the error is clear now, it could have the previous environment. But I know have a different error but i might not be related to your changes numpy.core.multiarray failed to import

not sure maybe you do pip install numpy again

ravi-kumar-pilla commented 3 weeks ago

Hi @jitu5 , The filter looks good but I see there are many reload requests. I saw we had this even before but is there a way to debounce these calls in watchfiles or a better way to handle file watching ?

I tested adding a change_test.json file under data/ folder which is ignored by git. Based on what I understood, the filter class should ignore the file change, instead it reloads Viz. Could you please confirm the behavior. Thank you

jitu5 commented 3 weeks ago

Hi @jitu5 , The filter looks good but I see there are many reload requests. I saw we had this even before but is there a way to debounce these calls in watchfiles or a better way to handle file watching ?

I tested adding a change_test.json file under data/ folder which is ignored by git. Based on what I understood, the filter class should ignore the file change, instead it reloads Viz. Could you please confirm the behavior. Thank you

@ravi-kumar-pilla Yes change_test.json should be ignored. I tested the same scenario and it was reloading viz. But when I checked the .gitignore file of demo-project it has two line related to data folder.

# ignore everything in the following folders
data/**
# except their sub-folders
!data/**/

Because of the second line with except condition, pathspec is not interpreting .gitignore the same way as git does. But pathspec library has specialised class GitIgnoreSpec. Now I moved to GitIgnoreSpec and its ignoring change_test.json file under data/ folder correctly.

kedro-org / kedro-viz