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

use pythonic way to read log #102

Closed
kungfoome opened this issue May 11, 2021 · 8 comments · Fixed by #164
Closed

use pythonic way to read log #102

kungfoome opened this issue May 11, 2021 · 8 comments · Fixed by #164

Comments

@kungfoome
Copy link

kungfoome commented May 11, 2021

I was trying to run this with WSL to see if that would work. It seems like there is an issue where you can't follow a file in WSL without the ---disable-inotify flag on tail. I tested this and added that option in the log consumer and it does work after that. I noticed there have been issues with Windows as well during log rotations. I'm not sure if switching over to a pythonic way of reading the debug file will solve this, but it seems like it would be better since you don't need differentiate between OSes either.

Not sure if remote would work with this either, so it would require a bit of testing.

if is_win_platform():
consume_command_args = ["powershell.exe", "get-content", expanded_user_log_path, "-tail", "1", "-wait"]
else:
consume_command_args = ["tail", "-F", expanded_user_log_path]
f = subprocess.Popen(consume_command_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
while self._is_running:
log_line = f.stdout.readline().decode(encoding="utf-8")
self._notify_subscribers(log_line)

Maybe something like: https://stackoverflow.com/questions/12523044/how-can-i-tail-a-log-file-in-python/53121178#53121178

Edit:

Initially something like this appears to work with little resources:

    def _consume_loop(self):
        expanded_user_log_path = self._log_path.expanduser()
        logging.info(f"Consuming log file from {expanded_user_log_path}")

        try:
            fp = open(expanded_user_log_path, "r")
            st_results = os.stat(expanded_user_log_path)
            st_size = st_results[6]
            fp.seek(st_size)

            while self._is_running:
                where = fp.tell()
                line = fp.readline()
                if not line:
                    time.sleep(1)
                    fp.seek(where)
                elif fp.tell() > os.path.getsize(expanded_user_log_path):
                    logging.info("Log rotated, opening latest log file")
                    fp.close()
                    fp = open(expanded_user_log_path, "r")
                    fp.seek(0,2)
                else:
                    self._notify_subscribers(line)
        except Exception as e:
            fp.close()
            logging.critical(f"Error consuming log file: {e}")

I can run it for a bit and see what happens during rotations as well. This seems to work the same for windows.

@martomi
Copy link
Owner

martomi commented May 11, 2021

Thanks @kungfoome! I see you have some very specific suggestions that are more suitable for a PR. You can fork the repository and submit a PR towards the dev branch here directly from your fork. Then it'll be easier to review and test the changes.

@pieterhelsen Check this out, maybe a fix for the Windows issues with log rotations.

@pieterhelsen
Copy link
Collaborator

Definitely worth a try and would be nice if it worked cross-platform.
My work on the log rotation issue is available here:
https://github.com/martomi/chiadog/tree/windows-rotation

It's tested on Linux local, Linux remote and Windows local, but I can't test Windows remote without setting up a dummy node

@pieterhelsen
Copy link
Collaborator

@kungfoome I just implemented your solution and one other, but did not get either of them to work reliably on my test setup due to the fact that open() locks up the file and as such makes my dummy LogWriter crash when it tries to rotate.

Now, it is possible that Chia uses another method that handles these rotations better, but I am currently unable to test this on my machine. Were you able to try this out further?

@martomi
Copy link
Owner

martomi commented May 17, 2021

@pieterhelsen
Copy link
Collaborator

I'll update my dummy logger to use this package and see if that alters behavior. From the description, it should!

@kungfoome
Copy link
Author

Yeah sorry. Haven't had time to follow up. I get the same behavior, although I don't think it's really because of open. That shouldn't create a lock on the file, but I think it's more so that it continues to read from the file and it doesn't have time to rotate. I think if you were to read x amount of bytes and set a timeout period, it would probably be ok.

But im interested in seeing if that other library works as well, as that would be much easier than trying to implement something from scratch.

@pieterhelsen
Copy link
Collaborator

pieterhelsen commented May 17, 2021

When using the Concurrent Log Handler it doesn't produce any errors, but reading the logfile in a pythonic still locks up the file, and prevents log rotation. (at least on Windows)

So it's a little more robust, but the issue persists.

@pieterhelsen pieterhelsen linked a pull request May 23, 2021 that will close this issue
@martomi
Copy link
Owner

martomi commented May 24, 2021

#164 added improvements, the discussion should continue in #72 if there are still issues

@martomi martomi closed this as completed May 24, 2021
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 a pull request may close this issue.

3 participants