getnikola / nikola

A static website and blog generator
https://getnikola.com/
MIT License
2.62k stars 450 forks source link

nikola auto 2.0 #3758

Open aknrdureegaesr opened 9 months ago

aknrdureegaesr commented 9 months ago

This is more an epic rather than a mere bug. I file this as a "bug" as it also reports several bugs.

Bugs: unstable tests/integration/test_dev_server.py

On several occasions, we have seen the tests/integration/test_dev_server.py misbehave

Expected state on close of this ticket: These instabilities are no longer observed.

Bugs: event loop not cleaned up

When under auto, rebuilding is triggered, errors may occur. These errors are handled nowhere. It is generally considered bad practice to never harvest exceptions.

Expected state on close of this ticket: By the time _excute finishes, the event loop should be empty, all awaitables "harvested" and, in particular, no pending exception left in Nirvana. There should be no explicit termination of the event loop, so tests can run several instances of auto.

Automated tests

By the time this ticket gets closed, there should be automated tests that perform a series of events like this:

It will probably a good idea to do these steps for several types of files (posts, pages, ...).

Implementation hint: For the output, a temporary directory can be used that is provided, and cleaned up, by pytest.

Bug: Pertinent conf changes don't restart the web server

Some lines in the site configuration contain information that is used to set up the web server of auto. When those lines in the conf file change, the web server should be shut down and restarted with the new information. This does not happen presently.

By the time this ticket gets closed: Either this bug is fixed, or it is at least documented via a comment line in the template that is used to generate a new conf file when a new site is initialized.

Deprecation: The way we use loop

Presently, nikola auto uses low-level primitives. The general recommendation is, to use more of the high-level stuff where feasible. In particular, we use asyncio.get_event_loop() to automagically create the event loop. But the documentation says about exactly our use:

Deprecated since version 3.12: Deprecation warning is emitted if there is no current event loop. In some future Python release this will become an error.

By the time this ticket gets closed, a serious attempt should have been made to use higher level stuff, such as asyncio.run(), and restrict the low-level stuff such as direct loop access to places where it is not easily avoided.

Race conditions schedule / cleanup

The watcher is taken down after the loop functionality is. This is a potential race condition: A last-minute change on the file system could trigger new building steps at a time when we do no longer want to service the loop.

Also, if many building steps are being processed and the loop is terminated in the heat of battle, it would be good to explicit cancel things that are going on. Presently, loop.call_soon_threadsafe is called and the return value thrown away, so it is not possible to cancel those actions.

By the time this ticket gets closed, these deficiencies are handled. - This is a special case of "event loop gets cleaned up".

Documentation of internal stuff as such

The classes inside the auto code are implementation details, not API. We may want to mention that in the class comments. Not entirely sure, as this could be construed to imply "if not mentioned (at other places), it is API".

Type hints (nice to have)

By the time this ticket gets closed, it would be good if the auto code has type hints for everything.

Kwpolska commented 9 months ago

Bug: Pertinent conf changes don't restart the web server By the time this ticket gets closed: Either this bug is fixed, or it is at least documented via a comment line in the template that is used to generate a new conf file when a new site is initialized.

Implementing config reload would be tricky, and a comment in the configuration file would not be very easy to notice, it could go somewhere in the manual or nikola help auto.

(PS. This might be nikola auto v3.0, since we’ve already had a major rewrite from some legacy stuff (wsgiref + ws4py) to asyncio + aiohttp.)