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

ignore files for quartdoc build -- watch #228

Merged
merged 8 commits into from
Aug 3, 2023
Merged

ignore files for quartdoc build -- watch #228

merged 8 commits into from
Aug 3, 2023

Conversation

hamelsmu
Copy link
Collaborator

This attempts to addresses #223 by adding files we might want to ignore

I wasn't able to repro #223 but I made a best effort based on the comments around ignoring __pycache__ which seems reasonable. I also added similar directories we wanted to ignore.

@machow @wch do you want to test this branch to see if it solves the issue?

@machow
Copy link
Owner

machow commented Jul 31, 2023

Thanks for this! Do you mind tweaking here (or in another PR) the try/except to do something like...

                except KeyboardInterrupt:
                    pass
                finally:
                    observer.stop()
                    observer.join()

It looks like when non- KeyboardInterrupt errors occur right now, they leave they end up not joining the handler

@hamelsmu
Copy link
Collaborator Author

Thanks for this! Do you mind tweaking here (or in another PR) the try/except to do something like...

done 85634c6

@machow
Copy link
Owner

machow commented Aug 1, 2023

It looks like it is still rebuilding multiple times.

image

I tested by:

  • running quartodoc build --watch from quartodoc's docs folder
  • changing quartodoc/renderers/md_renderer.py (just added a newline)

Do you mind taking another look? Thanks!

edit: I wonder if we need to ignore the __pycache__ directory itself (so the trailing slash throws it off), but have no idea based off of the very sparse watchdog docs 😭

@hamelsmu
Copy link
Collaborator Author

hamelsmu commented Aug 1, 2023

Wasn't able to repro

Steps I took

gh pr checkout 228
pip install -e .
cd docs
quartodoc build --watch

Then I add the following function to md_renderer.py

def foo(): pass

The following is emitted to the logs (no infinite loop)

Watching /Users/hamel/github/quartodoc/quartodoc for changes...
Rebuilding docs.  Detected: modified path : /Users/hamel/github/quartodoc/quartodoc/renderers/md_renderer.py

@hamelsmu
Copy link
Collaborator Author

hamelsmu commented Aug 1, 2023

Stuff about my environment:

Python 3.9.12
Quarto v1.4.224

watchdog                      3.0.0
click                         8.1.3
click-default-group-wheel     1.2.2
plum-dispatch                 1.7.4

@hamelsmu
Copy link
Collaborator Author

hamelsmu commented Aug 2, 2023

@machow this is ready for review. I limited the Queue size to 1 and also added a delay into the callback such that when there is a flood of events at the same time it will ignore ones that occur in near succession (to mitigate what happens when you change branches in git)

Copy link
Owner

@machow machow left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for working out all the watcher quirks. All this stuff has made docs workflow way smoother!

I left a small question about the order of printing (I noticed "Rebuilding ..." came after the build). Sorry for missing it when reviewing earlier 😓

new_file_info = self.get_file_info(event.src_path)
if self.is_diff(self.old_file_info, new_file_info):
self.callback()
self.print_event(event)
Copy link
Owner

Choose a reason for hiding this comment

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

I was thrown for a second seeing the Rebuilding message, after the build output. I wonder if putting it before might help people connect the triggering event with the build? WDYT?

For long builds, a message after might look like nothing's happening (could also sandwich the build with a pre "going to build" message, and post "just built" message)

Here is what my output looked like...

Watching /Users/machow/repos/quartodoc/quartodoc for changes...
2023-08-03 10:05:56,694 - quartodoc.autosummary - INFO - Generating blueprint.
2023-08-03 10:05:56,701 - quartodoc.builder.blueprint - INFO - Getting object for quartodoc:Auto
...
2023-08-03 10:05:56,798 - quartodoc.autosummary - INFO - Creating inventory file
2023-08-03 10:05:56,798 - quartodoc.autosummary - INFO - Creating inventory
2023-08-03 10:05:56,800 - quartodoc.autosummary - INFO - Writing sidebar yaml to api/_sidebar.yml
Rebuilding docs.  Detected: created path : /Users/machow/repos/quartodoc/quartodoc/a.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@machow I fixed this in 7593571

Thanks

Copy link
Owner

@machow machow left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for slogging through this!

@machow machow merged commit 7c76a48 into main Aug 3, 2023
4 checks passed
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