Skip to content

feat(cronjobs): add shared HTTP client wrapper. Refs #1214#1253

Open
Prasad8830 wants to merge 2 commits intoGreedyBear-Project:developfrom
Prasad8830:feat/http-client-wrapper-1214
Open

feat(cronjobs): add shared HTTP client wrapper. Refs #1214#1253
Prasad8830 wants to merge 2 commits intoGreedyBear-Project:developfrom
Prasad8830:feat/http-client-wrapper-1214

Conversation

@Prasad8830
Copy link
Copy Markdown

Description

This PR introduces a shared HTTP client wrapper (HttpClient) based on requests.Session for our cronjob fetchers. This represents the first step towards standardising how cron jobs handle timeouts, retries, and errors, as discussed in #1214.

The wrapper automatically applies a default 10-second timeout, implements an exponential backoff retry strategy for standard server errors/rate limits, and consistently enforces raise_for_status(). Full unit tests for this wrapper have been included.

Note: This PR only introduces the wrapper and its tests. Subsequent PRs will migrate tor_exit_nodes, whatsmyip, and the remaining endpoints to use this new client.

Related issues

Type of change

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Chore (refactoring, dependency updates, CI/CD changes, code cleanup, docs-only changes).

Checklist

Formalities

  • I have read and understood the rules about how to Contribute to this project.
  • I chose an appropriate title for the pull request in the form: <feature name>. Closes #999
  • My branch is based on develop.
  • The pull request is for the branch develop.
  • I have reviewed and verified any LLM-generated code included in this PR.

Docs and tests

  • I documented my code changes with docstrings and/or comments.
  • I have checked if my changes affect user-facing behaviour that is described in the docs. If so, I also included an update to the wiki in the description of this PR.
  • Linter (Ruff) gave 0 errors. If you have correctly installed pre-commit, it runs these checks and makes these adjustments on your behalf.
  • I have added tests for the feature/bug I solved.
  • All the tests gave 0 errors.

GUI changes

Ignore this section if you did not make any changes to the GUI.

  • I have provided a screenshot of the result in the PR.
  • I have created new frontend tests for the new component or updated existing ones.

Copy link
Copy Markdown
Member

@regulartim regulartim left a comment

Choose a reason for hiding this comment

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

Hey @Prasad8830 ! Nice work overall! :) A few things though:

  • The issue was also about standardized logging but I can see no logging here. Please add a sensible amount of logging and also be thoughtful with the log levels.
  • The retry behavior is not actually tested. Please add some tests for that, especially for the POST + retry case.

Comment thread greedybear/cronjobs/http_client.py
@Prasad8830
Copy link
Copy Markdown
Author

Hey @Prasad8830 ! Nice work overall! :) A few things though:

  • The issue was also about standardized logging but I can see no logging here. Please add a sensible amount of logging and also be thoughtful with the log levels.
  • The retry behavior is not actually tested. Please add some tests for that, especially for the POST + retry case.

Thank you for bringing that to my attention! I will make the necessary corrections.

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