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

fix: logging behavior #22

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: logging behavior #22

wants to merge 7 commits into from

Conversation

5u6r054
Copy link

@5u6r054 5u6r054 commented Feb 15, 2025

Fix Package Logging Behavior

This PR improves the logging behavior of the package, particularly for the tweet validation functionality. The changes make the package more respectful of the parent application's logging preferences and add better error handling.

Key Changes

Logging Improvements

  • Made logging configurable and opt-in by default
  • Moved detailed logs to DEBUG level
  • Added clear ✅/❌ indicators for validation results
  • Removed duplicate logging
  • Fixed log propagation to prevent unwanted logs in parent apps

Error Handling

  • Added centralized request handling with timeouts
  • Improved rate limit handling with exponential backoff
  • Better handling of connection errors and timeouts
  • Added proper null checks for response objects
  • Moved error details to DEBUG level

Code Quality

  • Added type hints
  • Split long functions into smaller ones
  • Added helper method for data extraction
  • Improved validation logic
  • Fixed linting issues

Example Log Output

Before:

validator | Error obtaining guest token: 429 Client Error: Too Many Requests
validator | Failed to obtain guest token
validator | Tweet data could not be fetched for tweet ID 1234567
validator | Tweet validation failed: https://x.com/i/status/1234567

After:

validator | INFO | ✅ Tweet validation passed: https://x.com/i/status/1234567
validator | INFO | ❌ Tweet validation failed: https://x.com/i/status/1234567

Testing

The changes have been tested with:

  • Rate limit scenarios
  • Connection errors
  • Timeout conditions
  • Valid and invalid tweets
  • Parent application integration

Production Testing

These changes have been successfully tested in production on the Subnet 42 validator, handling high-volume tweet validation with improved rate limiting and cleaner logging output. The validator maintained stable operation with the new logging behavior while processing multiple concurrent validation requests.

Breaking Changes

None. The public API remains unchanged, only the logging behavior has been modified to be less intrusive.

- Replace loguru with standard Python logging

- Make logging configurable and opt-in by default

- Fix log levels for validation results

- Improve code organization and type hints

- Add data extraction helper method

- Clean up validation logic
@5u6r054 5u6r054 requested review from theMultitude and grantdfoster and removed request for theMultitude February 15, 2025 04:07
- Add explicit handling for ServerDisconnectedError\n- Log disconnection errors at debug level only\n- Prevent error propagation to parent application logs\n- Maintain consistent retry behavior with other network errors
- Remove ServerDisconnectedError import as it's not available in all requests versions\n- Handle server disconnections using the more general ConnectionError\n- Update error message to indicate possible server disconnection\n- Maintain same retry behavior for all connection issues
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.

1 participant