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

Set log download attachment name to crawl_id-logs.jsonl #1280

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

tw4l
Copy link
Member

@tw4l tw4l commented Oct 12, 2023

Fixes #1271

@tw4l tw4l requested review from Shrinks99 and ikreymer October 12, 2023 16:13
@ikreymer
Copy link
Member

For me, on mac, opening a .jsonl file leads to an error, while opening a .log or .txt file works (opens in a text viewer).
I think maybe we should stick with one of these, perhaps .log, as these are more standard

@tw4l
Copy link
Member Author

tw4l commented Oct 14, 2023

For me, on mac, opening a .jsonl file leads to an error, while opening a .log or .txt file works (opens in a text viewer). I think maybe we should stick with one of these, perhaps .log, as these are more standard

Hm, the inital impetus from @Shrinks99 for the change was wanting syntax highlighting in a text editor based on the extension, but I see your point here. Maybe .log is fine? Hank, what do you think?

@Shrinks99
Copy link
Member

Shrinks99 commented Oct 14, 2023

It's not only about syntax highlgihting (though that is a bonus), it's more about actually conveying the type of data that is in the file... as that's what file types are all about! Given that our logs are structured data and not printed messages (where .txt or .log would be a good choice) I think we should name our file extension accordingly?

That said, I'm pretty sure most programs don't do that and just name things .log. Kind of a bummer? Not a fan. This probably stems from an overall lack of standardization in logging formats (Apple actually has their own proprietary .asl format that they use in Console for macOS system logs), would rather we set a good example but if I'm to be overruled on this one I guess .log would be a better choice than .txt, at least it will open in your log viewer of choice.

Finally, to be perhaps a little too snarky... The information within the log file likely won't help anyone who gets stuck figuring out how to open a .jsonl file 🙃. To solve for this, I'd like to eventually attach actionable next steps to the log viewer within the UI.

@ikreymer
Copy link
Member

I think we want to err on side of users being able to double-click on downloaded logs and open in their default text editor, to help with support issues down the road. (Users who'll parse the jsonl will likely do it via command line or via api where this doesn't matter as much). For that, lets go with the familiar .log for now.

@ikreymer ikreymer merged commit 2383b0d into main Oct 14, 2023
@ikreymer ikreymer deleted the issue-1271-logs-extension branch October 17, 2023 05:04
tw4l added a commit that referenced this pull request Oct 18, 2023
Fixes #1271
Using .log for now due to broader support for opening with default viewers

---------
Co-authored-by: Ilya Kreymer <[email protected]>
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 this pull request may close these issues.

[Bug]: Logs download as TXT when they are in fact JSONL files
3 participants