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

wait option does not change the reload cycle #2285

Closed
AnjoMan opened this issue Jan 12, 2021 · 9 comments
Closed

wait option does not change the reload cycle #2285

AnjoMan opened this issue Jan 12, 2021 · 9 comments
Milestone

Comments

@AnjoMan
Copy link

AnjoMan commented Jan 12, 2021

I wrote a plugin that generates files in docs_dir, the general structure of which is:

class AutoDocumentExtension(mkdocs.plugins.BasePlugin):
    def on_pre_build(self, config):
        # generate some markdown and insert it into the docs tree
        with open(Path(config["docs_dir"]) / "special_folder" / "autogenerated_docs.md", "w") as f:
            f.write("# Automatic High-Quality Content!")

I have been trying to use the wait keyword argument (e.g. `mkdocs serve --wait 60) to keep the live-reload feature from watching my auto-generated files before they are updated after a reload, since this would cause a build loop. However, it seems that the wait number has no effect -- as noted in #2284, even if i make the delay extremely long (50,000 seconds) there is no effect on the cycle time.

i think this may come down to an issue with the livereload package, although it provides the delay option there appears to be no unit tests and its not clear that the feature works. Perhaps it should be rolled back until we can find a better approach for achieving this wait?

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Feb 6, 2021

Following up here as asked in #2093.

From what I understand, the situation I describe in #2093 (and that is originally described in #2061) is not the same as what is described here.

OP here seems to be auto-generating a file early in the build process. Apparently it triggers the livereload watcher, so the docs are immediately rebuilt after the first build. And then again since the second build also triggered the watcher. This is why OP mentions a build loop. Am I correct @AnjoMan? If I am, then yes, a delay here is the solution: discard watcher events during the first N seconds. This way the file auto-generation does not trigger a rebuild, and we do not enter a build loop.

But even if we use a delay, multiple events occurring after that delay will trigger multiple rebuilds, instead of just one (i.e. a batching update, mentioned by @squidfunk in #2061). What if a watched directory generates 1000 events in the span of 10 seconds? We have to set a 10 seconds delay? Then for every single change we do (for example saving a Markdown page), we'll have to wait 10 seconds before the reload? I'm just asking to make sure I correctly understand the situation.

So if I understand correctly, we need another option to wait until a specified period of inactivity is reached, i.e. only trigger the build once no changes were detected since the last N seconds. In my case, even 0.2 seconds would be enough to reduce the number of rebuilds from events times to one time only.

I do agree that we must check if the delay feature works in upstream anyway 🙂

@squidfunk have you been able to workaround the multiple rebuild you describe in #2061?

@waylan
Copy link
Member

waylan commented Feb 6, 2021

First of all, all MkDocs does is accept the --wait argument, validate that it is a valid value (an integer), and then pass it directly to python-livereload. I'm not interested in developing or maintaining anything more complex than that.

So if I understand correctly, we need another option to wait until a specified period of inactivity is reached, i.e. only trigger the build once no changes were detected since the last N seconds.

This was my understanding of what python-livereload's wait feature did. If it doesn't, then the feature is not useful to us. What purpose would the wait even serve?

Finally, if the above is correct and python-livereload does not offer the feature we want/need, then the appropriate way to get that feature is to add the feature to python-livereload. Once python-livereload supports the feature, then we can include support for it.

@pawamoy
Copy link
Sponsor Contributor

pawamoy commented Feb 7, 2021

This was my understanding of what python-livereload's wait feature did.

Perfect, sorry for thinking we understood it differently! Then I'll try to find time to investigate on python-livereload's side! Thanks @waylan

waylan added a commit to waylan/mkdocs that referenced this issue Apr 8, 2021
This reverts commit f73f221.

It seems that the `wait` flag does not interupt the file watcher,
but simply delays builds. Therefore, a plugin which writes to the
`docs_dir` will still result in an infinite loop. That being the
case, the `wait` flag is not useful to us. See mkdocs#2285.
@oprypin
Copy link
Contributor

oprypin commented Apr 8, 2021

Disregarding the title of this issue, the exact described use case (writing files into docs dir during the build of that docs dir) is not a good approach and, I'd say, should not be catered towards.
There will be other issues, such as the file accidentally overwriting real files, or lingering there after an interruption and accidentally being committed, and probably more.
MkDocs' behavior isn't even wrong there.

Instead one can write to a separate temporary directory and add that file into the Files structure.
https://github.com/oprypin/mkdocs-gen-files/blob/08d6e91924af3a67031f30d925de12e893c5804b/mkdocs_gen_files/editor.py#L42-L47

#2061 is a separate issue, and as of now it is indeed not solved, so probably should be reopened.

@waylan
Copy link
Member

waylan commented Apr 8, 2021

(writing files into docs dir during the build of that docs dir) is not a good approach and, I'd say, should not be catered towards.

@oprypin you make a good point. In fact, that was my response the first few times this came up. However, after repeated requests, I (incorrectly) thought that livereload's wait flag would provide a solution. As it was an easy fix (I thought), I went forward with it.

#2061 is a separate issue, and as of now it is indeed not solved, so probably should be reopened.

Actually, as #2061 specifically applies to themes, it is addressed in #2094. That said, when the --watch-theme option is used by a theme dev, the issue in #2061 still exists.

@oprypin
Copy link
Contributor

oprypin commented Apr 8, 2021

#2061 only mentions themes as one example, but the issue applies to any file. I'll comment there.

@AnjoMan
Copy link
Author

AnjoMan commented Apr 8, 2021

Instead one can write to a separate temporary directory and add that file into the Files structure.

Is the idea here to implement the on_files event and just create extra File objects? That seems pretty straightforward, but its currently hard to know how to do that because the Files structure isn't documented. The approach i did (just make the files and put them where your static files are) is the obvious thing to do.

Maybe if there are a lot of requests about doing this kind of thing, we just need to add a bit in the plugin documentation about how on_files works?

I'll try reimplementing my plugin using the suggestion from @oprypin and see if i could come up with a simple explanation for how it works

waylan added a commit that referenced this issue Apr 8, 2021
This reverts commit f73f221.

It seems that the `wait` flag does not interupt the file watcher,
but simply delays builds. Therefore, a plugin which writes to the
`docs_dir` will still result in an infinite loop. That being the
case, the `wait` flag is not useful to us. See #2285.
@oprypin
Copy link
Contributor

oprypin commented Apr 8, 2021

@AnjoMan

BTW instead of implementing this logic, you can very likely just use the plugin itself :)
https://github.com/oprypin/mkdocs-gen-files

Is the idea here to implement the on_files event and just create extra File objects?

Yes. It receives the Files instance, which itself is an iterable of File objects

def __iter__(self):
return iter(self._files)
Then you create a new modified one

def on_files(self, files : mkdocs.structure.files.Files, config) -> mkdocs.structure.files.Files:
    file_list = list(files)
    file_list.append(File(......))
    return mkdocs.structure.files.Files(file_list)

@waylan
Copy link
Member

waylan commented Apr 8, 2021

the Files structure isn't documented

Yes, we know. In fact, adding that documentation is part of #1629.

But, yes @oprypin is correct. Using the on_files event to add File objects is the correct approach for your use case.

In any event, there is no actionable item in this issue that we will act on that is not already the subject of another issue (#2021 has been reopened). Therefore, I am closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants