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

Formatter handling improvements and extending python compatibility #13

Conversation

novama
Copy link
Collaborator

@novama novama commented Aug 26, 2024

Hi @xente!
I want to thank you for your work doing this great tool!

I would appreciate a lot if you consider these proposed changes to be merged.

The main changes here are:

  • All the changes from Pull Request Formatter handling improvements #12 are added here.

    • Added fallback to the current time in nanoseconds if the timestamp is missing or invalid in the stream's append_value() function. (This prevent a failure when the formatter does not include the 'timestamp' key)
      -Reorganized the sequence of arguments in the LokiLoggerHandler constructor (useful change when loading logging configuration from files using logging.config.fileConfig())
    • *Possible breaking change*: The arguments labelKeys and defaultFormatter in the LokiLoggerHandler constructor were renamed as label_keys and default_formatter for consistency with the naming conventions.
    • Improved docstrings and comments for better readability and maintenance.
    • Added tests for custom formatters, empty buffer handling.
  • For this PR all the scripts were reviewed, adjusted and tested to be compatible with Python 2.7 and keep the compatibility with Python 3.x.
    the purpose of this is to make it possible to use the library in Jython applications, which unfortunately currently only supports Python 2.7

  • All tests are green.

@novama novama mentioned this pull request Aug 26, 2024
1 task
timeout=10,
compressed=True,
defaultFormatter=LoggerFormatter(),
additional_headers=dict()
loguru=False,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we only have 2 formatters but i want to be able to add more. In this way we are not flexible to use more in the future.

It is ok in this way for the moment but in the future it will be needed to review.

@xente xente merged commit 2a8345c into xente:main Nov 18, 2024
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.

2 participants