Reconsider HttpClient interface · Issue #874 · apify/crawlee-python · GitHub
Skip to content

Reconsider HttpClient interface #874

Open
Open
Reconsider HttpClient interface#874
Labels
debtCode quality improvement or decrease of technical debt.solutioningThe issue is not being implemented but only analyzed and planned.t-toolingIssues with this label are in the ownership of the tooling team.
Milestone
2.0

Description

@janbuchar
janbuchar
opened on Jan 6, 2025

As of now, the python BaseHttpClient look like this:

crawlee-python/src/crawlee/http_clients/_base.py

Line 55 in beac9fa

It has two methods, send_request and crawl. This is the first iteration of decoupled HTTP clients.

Later on, we refactored the JS version to use this one:

https://github.com/apify/crawlee/blob/f912b8b06da2bc4f3f3db508cc39c936a5c87f23/packages/core/src/http_clients/base-http-client.ts#L179

It also has two methods, sendRequest and stream. Unlike the python version, the signatures of those methods match quite well. It is worth noting that the two serious attempts at implementing this interface (so far) both couldn't manage to implement stream correctly. Although we could probably live without it in the most common case, streaming is paramount for downloading files (potentially large ones), which is a use case that we want to support.

We should simplify this interface and make it look the same in both versions. Any thoughts on how to achieve that? @vdusek @B4nan @barjin... and whoever else wants to chat 🙂

This will be a breaking change.

Activity

  1. janbuchar
    added
    debtCode quality improvement or decrease of technical debt.
    solutioningThe issue is not being implemented but only analyzed and planned.
    on Jan 6, 2025
  2. github-actions
    added
    t-toolingIssues with this label are in the ownership of the tooling team.
    on Jan 6, 2025
  3. B4nan
    self-assigned this
    on Jan 13, 2025

  4. B4nan commented on Jan 15, 2025

    @B4nan
    B4nan
    on Jan 15, 2025
    Member

    I like the JS version, but given many HTTP clients won't support streaming or it will be challenging to implement, I think we could have a default implementation of the stream method as part of the base class. So in JS instead of having an interface, it would be an abstract class with the stream method implemented with a fallback like we do with the curl-impersonate client.

    I am not sure why there is that crawl method in python, if we don't support streaming there at all, ideally we would drop it (and implement a proxy at the call site if we need a different signature). Feels weird that crawler.run and send_request would use something else. And we should surely think of how to support streaming there too, but that's another story.

  5. vdusek commented on Jan 16, 2025

    @vdusek
    vdusek
    on Jan 16, 2025
    Collaborator

    I am not sure why there is that crawl method in python, if we don't support streaming there at all, ideally we would drop it (and implement a proxy at the call site if we need a different signature). Feels weird that crawler.run and send_request would use something else. And we should surely think of how to support streaming there too, but that's another story.

    Mostly historical reasons - the first attempt of implementation of HTTP clients. The crawl method is being called from the crawler.run, and send_request is being called from the send_request context helper. They have a slightly different APIs, but I believe they could be unified.

    I like the JS version, but given many HTTP clients won't support streaming or it will be challenging to implement, I think we could have a default implementation of the stream method as part of the base class. So in JS instead of having an interface, it would be an abstract class with the stream method implemented with a fallback like we do with the curl-impersonate client.

    I like the refactored version in JS as well and agree with the streaming. IMO it's acceptable if not all clients support streaming. A default implementation of the stream method in the base class, with a warning when it's not fully supported, would be okay fallback.

  6. janbuchar commented on Jan 16, 2025

    @janbuchar
    janbuchar
    on Jan 16, 2025
    CollaboratorAuthor

    I like the JS version, but given many HTTP clients won't support streaming or it will be challenging to implement, I think we could have a default implementation of the stream method as part of the base class. So in JS instead of having an interface, it would be an abstract class with the stream method implemented with a fallback like we do with the curl-impersonate client.

    I like the refactored version in JS as well and agree with the streaming. IMO it's acceptable if not all clients support streaming. A default implementation of the stream method in the base class, with a warning when it's not fully supported, would be okay fallback.

    This is preferrable in the arguably common case when streaming cannot be implemented. I can't shake the feeling that stream-based implementations should be the default because it's easy to read a stream into memory when needed, but doing it the other way around nullifies the benefit of streams. Plus it's not nice to require people to implement both a streaming and non-streaming version.

  7. B4nan
    removed their assignment
    on Dec 4, 2025

  8. B4nan commented on Dec 4, 2025

    @B4nan
    B4nan
    on Dec 4, 2025
    Member

    I guess that one can be closed after all the refactoring in v1? Feel free to reopen if not.

  9. B4nan
    closed this as completedon Dec 4, 2025

  10. vdusek commented on Dec 4, 2025

    @vdusek
    vdusek
    on Dec 4, 2025
    Collaborator

    I'm not sure. We didn't do much on the HTTP clients before the v1 release, and there are still send_request and crawl in Python and send_request and stream in the TS version.

    cc @janbuchar

  11. janbuchar commented on Dec 4, 2025

    @janbuchar
    janbuchar
    on Dec 4, 2025
    CollaboratorAuthor

    Yeah, crawl is kinda unnecessary now and the HttpClient interface should be cleaned up - ideally to the point that there is a single stream-enabled method. Let's keep this open.

  12. janbuchar
    reopened this on Dec 4, 2025
  13. vdusek
    added this to the 2.0 milestone on Dec 4, 2025

  14. vdusek commented on Dec 4, 2025

    @vdusek
    vdusek
    on Dec 4, 2025
    Collaborator

    This is evolving in Crawlee JS v4 (fetch, sendRequest, applyCookies, ...); let's unify this in Crawlee Python v2.0.

  15. vdusek commented on Jul 15, 2026

    @vdusek
    vdusek
    on Jul 15, 2026
    Collaborator

    Another discussion relevant to this topic: #2043 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    debtCode quality improvement or decrease of technical debt.solutioningThe issue is not being implemented but only analyzed and planned.t-toolingIssues with this label are in the ownership of the tooling team.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions