Fixed an issue where the tail would fail in case of file rename and replace as in log rotation scenario. #22
Conversation
…eplace as in log rotation scenario.
| @@ -234,9 +245,18 @@ def follow(file, delay=1.0): | |||
| >>> f.flush() | |||
| >>> next(generator) | |||
| 'Line 2' | |||
| >>> os.rename("test_follow.txt", "test_follow_1.txt") | |||
There was a problem hiding this comment.
This is a separate test case. The doctests are meant to act as documentation as well as tests. When we start adding new test cases, they should be added as separate pytests.
| >>> os.rename("test_follow.txt", "test_follow_1.txt") | ||
| >>> f = open('test_follow.txt', 'w') | ||
| >>> fo = open('test_follow.txt', 'r') | ||
| >>> generator = follow(fo) |
There was a problem hiding this comment.
This doesn't seem to be testing the case of "when a file currently being opened is renamed" while following.
This is starting a new follow(fo) with the new file. What I would expect is the original generator is used without needed to create a new follow.
| @@ -160,7 +161,17 @@ def follow(self, delay=1.0): | |||
| """ | |||
| trailing = True | |||
|
|
|||
| initial_inode = os.stat(self.file.name).st_ino | |||
There was a problem hiding this comment.
tailer is meant to work on streams as well. So an io.StringIO should work too. io.StringIO does not have a .name parameter so this would fail.
| @@ -160,7 +161,17 @@ def follow(self, delay=1.0): | |||
| """ | |||
| trailing = True | |||
|
|
|||
| initial_inode = os.stat(self.file.name).st_ino | |||
| file_path = self.file.name | |||
|
|
|||
| while 1: | |||
There was a problem hiding this comment.
Use while True:. There's no need to optimize to using an int here. I see while 1 is used in other places. I'd consider that a bad practice now.
| self.file = open(file_path) | ||
| initial_inode = os.stat(self.file.name).st_ino | ||
| except FileNotFoundError: | ||
| continue |
There was a problem hiding this comment.
This will get stuck in a tight infinite loop if the file does not exist. We need a non-blocking wait. The easiest way to do this is to introduce a time.sleep(delay) if the file does not exist.
See comments on https://github.com/six8/pytailer/pull/14/files#r1167325214
We want to mimic what GNU tail is doing.
With GNU tail, the -f option will follow the file by inode. So if the file is renamed, it will continue to follow the file at the new name. We want to continue that behavior.
GNU tail adds a -F option, which is an alias for --follow=name --retry
Note: In --follow=name, name is a constant value. The options are name or descriptor with descriptor being the default.
See the GNU tail docs for more info https://man7.org/linux/man-pages/man1/tail.1.html
The following tests needed to be added. Start with the tests, then the implementation will be more obvious:
- If using
-fand the file is renamed, it continues to follow the file at the new name and ignores a file at the old name. - If using
-F(or--follow=name --retry) and the file is renamed, ignore the new name file and follow the old name file.
This means adding a new --retry and -F option and making it so --follow has 3 states:
--follow--follow=name--follow=descriptor
You can see how GNU tail is implemented at https://github.com/coreutils/coreutils/blob/master/src/tail.c. It uses inotify, which we can't use because inotify is not part of the Python standard library, and the intention is to keep tailer pure-python and only depend on the standard library.
Fixed an issue where the tail would fail in case of file rename and replace as in log rotation scenario.