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

asctime handling #17

Open
nhairs opened this issue Jun 4, 2024 · 0 comments
Open

asctime handling #17

nhairs opened this issue Jun 4, 2024 · 0 comments
Labels
open for discussion Community feedback is wanted

Comments

@nhairs
Copy link
Owner

nhairs commented Jun 4, 2024

Currently using the asctime field will produce a string that is formatted separately from the JSON encoders.

By default it is produced by logging.Formatter.formatTime which uses converter=time.localtime and datefmt.

It feels like instead it should be a datetime encoded by the JSON encoder. However doing this would raise other questions like:

  • What do we do with the datefmt argument?
  • What do we do with the converter attribute?
  • Do we remove the timestamp argument?
    • This could be converted to an alias that adds asctime to required_fields and "asctime": "timestamp" to rename_fields. Though then what do we do if asctime and timestamp are used?
    • I'll note that there doesn't appear to be any reasoning for why timestamp was added included in the commit that added it. Searching old issues doesn't surface anything either.
  • Do we support time zones that aren't UTC?
  • What about users still using asctime, datefmt, and converter?
    • Searching old issues - it looks like there people who are using these to control the output of asctime.

As such any decision to move to using the JSON encoder or changing / removing timestamp is likely a breaking change which we probably want to avoid for as long as possible.

Another approach would be to set "saner" defaults for formatTime or (datefmt and converter). This could be considered a non-breaking change (anyone using defaults would experience the change, but anyone using custom shouldn't see breakages).

Another approach would be to use some kind of "opt-in" keyword argument that toggles new behaviour and otherwise uses old behaviour.

Perhaps in the short term all we do is update the docs to suggest that using timestamp over asctime.

Opinions welcome

@nhairs nhairs added the open for discussion Community feedback is wanted label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open for discussion Community feedback is wanted
Projects
None yet
Development

No branches or pull requests

1 participant