-
Notifications
You must be signed in to change notification settings - Fork 1k
logging.FileHandler
not adding new lines to log file
#628
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
Comments
To be fair |
On many/most micropython ports the filesystem write operations go via a ram cache to ensure efficient use of flash blocks and to speed up overall operations. In many cases flash write is very slow, so you don't want to do it more often than necessary. The flush should happen occasionally automatically, whenever a flash block worth of logging has been written, out wherever any other file on the device gets written to. I think this should be there most efficient way for logging to work, however yes it runs the risk of losing the most recent logging data in the event of reset or power failure. How are you reading the log file while the application is running? |
This is how I read the file: from microdot_asyncio import Microdot
app = Microdot()
@app.get("/api/logs")
async def logs(request):
def generate_logs():
with open(log_file, 'r') as f:
for line in f:
yield line
return generate_logs() It's just for checking the logs quickly, I'm not using the log file in another function. |
Ah yep that makes sense! Your subclass is arguably the correct way to do this to flush immediately, when the logging is minimal and flush to disk doesn't slow things too much You could also just expose the flush function from the logging file handler and call it in the microdot route handler just before reading too, though that would slow down the request instead. It would be good to document things like this somewhere, the wiki is probably the right place! |
Thank you for your help. I think for now I'm gonna stick with the subclass, even if it means also having to copy paste Maybe we could add an |
You could override just Filrhander and include an emit function there which calls flush then Adding options like that is unfortunately a difficult thing to balance; while it feels useful for micropython, it breaks compatibility with cpython stdlib which is an overall goal for everything in the stdlib library here. |
You're right, I don't have to subclass Just so we're clear, CPython's |
To be honest I don't know for certain if this was a conscious design choice here, but I've had experience with very slow logging from flushing every line so it makes sense to me. Do you know if "flush in every line" is documented behaviour for cpython, or just happens to be the way it appears to work? |
Which old code are you referring to ? The old logger did Not have any defined Handlers, and it didn't call I agree with @andrewleech calling flush after every single write on MicroPython is very bad, would also like to add that on any modern OS/PC a That said, I only really care about API compatibility, if you want to submit a PR to fix it somehow without breaking the API go ahead, I think adding an arg might be okay, but it really depends on the maintainers though. Alternatively, it was mentioned more than once that there will be an extension package called |
@andrewleech I don't see anything explicitly mentioning a flush for every message in the CPython documentation. It was just an observation from reading the source code. @iabdalkader I'm referring to the code "adapted from CPython that I was previously using". Sorry if I wasn't clear enough. There was indeed no defined handlers before. Thanks for the explanation, I didn't know that about MicroPython. Right now the behaviour of If it's safer to leave Andrew pointed me to the wiki but I would expect this to be in the official docs. Unfortunately I don't think this is possible as these are two separate repos, and also because the modules here are updated separately from the "core". |
Heh, it's hard enough to match cpython stdlib documented API of packages let alone starting to fill in undocumented side effects of implementation ;-) As said previously, adding a flush to every line would have negative performance side effects for many users, so it would be bad to make this standard behaviour here. To be fair I only know this because I was helping on a project a couple of years ago with a serious bug where it was locking up at weird times... it turned out the bug was caused by their custom logging handler flushing every line! I don't know of anyone offering the time / effort required to document all the packages here, current intentions as I understand it is to match cpython stdlib API enough for their documentation to be "close enough" - but yes that means currently users need to read through the code to see which functions are written and which are missing. "The code is the documentation" so to speak. With regard to a Yes it'd be fantastic to have this sort of thing all covered in detailed documentation... pr's are always welcome ;-) |
Just tried the updated
logging
module to replace theStreamHandler
andFileHandler
adapted from CPython that I was previously using. Thank you @iabdalkader for this update.StreamHandler
works as expected, however I noticed that I wasn't getting any new lines in my log file.I looked at the previous code and saw that there's a
flush
after thewrite
inStreamHandler.emit
.Using this subclass for
FileHandler
, it works like before:Is there a reason for not doing a
flush
inemit
, maybe something specific to MicroPython that I'm not aware of?MicroPython v1.19.1-915-g2bcd88d55 on 2023-03-02; Raspberry Pi Pico W with RP2040
logging
version:0.5
installed withmip
The text was updated successfully, but these errors were encountered: