duffelhq / duffel-api-python

Python client library for the Duffel API
https://duffel.com/docs
MIT License
16 stars 7 forks source link

Suggestion: use of an async request framework (as replacement for `requests`) #306

Open cglacet opened 1 year ago

cglacet commented 1 year ago

Is your feature request related to a problem? Please describe. The issue is that many (most?) modern python web frameworks are working in an asynchronous fashion (ASGI). The current API you are offering is purely synchronous which make it "impossible" to use with such frameworks.

Describe the solution you'd like Most of the time I'm using aiohttp, its interface is very close to requests which makes it a good candidate.

Describe alternatives you've considered I'm currently re-writing it for myself.

Remark

Also, that's a bit related so I will answer this remark from your code:

# duffel_api/client.py
class Duffel:
    """Client to the entire API"""

    def __init__(self, **kwargs):
        # TODO(nlopes): I really don't like how I've built this- we shouldn't keep
        # instantiating the HttpClient through class inheritance.
        # We should (maybe!) have a singleton pattern on HttpClient and use
        # composition in all of these instead.

        # Keep this as we use it when doing the lazy-evaluation of the different
        # clients
        self._kwargs = kwargs

I think it's not too much of a problem, the real problem is underlying. You are creating one session per init (and therefore per inherited class), the code looks a bit like this:

# duffel_api/http_client.py
class HttpClient:
    def __init__(self, access_token=None, api_url=None, api_version=None, **settings):
        self.http_session = Session()

I think you should spawn a single HTTP session for all APIs (unless there is a good reason not to). Then, you could use that unique session in all subclasses (passing it as argument to the HTTpClient). You don't even need to have a singleton for that an can simply use a lazy property directly in Duffel client, like so (the code is already using aiohttp because I've already rewrote this part):

# duffel_api/client.py
class Duffel:
    _session: Optional[aiohttp.ClientSession] = None

    def __init__(self, **kwargs):
        self._kwargs = kwargs
        self._session = None

    @property
    def http_session(self):
        if self._session is None:
            self._session = aiohttp.ClientSession()
        return self._session

    @lazy_property
    def aircraft(self):
        """Aircraft API - /air/aircraft"""
        return AircraftClient(**self._kwargs, session=self.http_session)
# duffel_api/http_client.py
class HttpClient:
    _session: aiohttp.ClientSession

    URL = "https://api.duffel.com"
    VERSION = "v1"

    def __init__(
        self,
        *,
        session: aiohttp.ClientSession,
        access_token: str = None,
        api_url: str = None,
        api_version: str = None,
        **settings,
    ):
        self._api_url = api_url or HttpClient.URL
        self._api_version = api_version or HttpClient.VERSION

        if not access_token:
            access_token = os.getenv("DUFFEL_ACCESS_TOKEN")
            if not access_token:
                raise ClientError("must set DUFFEL_ACCESS_TOKEN")

        self._headers = {
            "Authorization": f"Bearer {access_token}",
            "User-Agent": f"Duffel/{self._api_version} duffel_api_python/{version()}",
            "Accept": "application/json",
            "Duffel-Version": self._api_version,
        }
        self._settings = settings
        self.http_session = session

    async def _http_call(self, endpoint, method, query_params=None, body=None):
        request_url = self._api_url + endpoint
        request_args = dict(params=query_params, json=body, headers=self._headers, **self._settings)
        async with self.http_session.request(method, request_url, **request_args) as response:
            if response.status_code in {http_codes.ok, http_codes.created}:
                try:
                    return response.json()
                # etc.

This way, all HTTP requests will use the same session (single TCP handshake).

You could also directly offer the enduser the opportunity to start the session on its own (for example when the server starts).

cglacet commented 1 year ago

Here is how it would look like:

client = Duffel(access_token="duffel_test_Utbb-kmsFGL44mi0Ixz8-jOpQzS7OmlkGuxfxX-F4rQ")
client.start() # TCP handshake, once and for all

offer_requests = client.offer_requests.list()
async for offer_request in offer_requests:
    print(offer_request.id)

And with a more complete example (translating the one linked in the documentation):

destination = "CDG"
origin = "JFK"
departure_date = "2023-12-01"

slices = [
    {
        "origin": origin,
        "destination": destination,
        "departure_date": departure_date,
    },
]
offer_request = await (
    client.offer_requests.create()
    .passengers([{"type": "adult"}])
    .slices(slices)
    .return_offers()
    .execute()
)
offers = offer_request.offers

for idx, offer in enumerate(offers[:5]):
    print(
        f"{idx + 1}. {offer.owner.name} flight departing at "
        + f"{offer.slices[0].segments[0].departing_at} "
        + f"{offer.total_amount} {offer.total_currency}"
    )

offer_index = 0

given_name = "Christian"
family_name = "Glacet"
dob = "1986-07-31"
title = "mr"
gender = "m"
phone_number = "+33624000097"
email = "xxxx@gmail.com"

print(f"\nHang tight! Booking offer {offer_index}...")

selected_offer = offers[int(offer_index) - 1]
payments = [
    {
        "currency": selected_offer.total_currency,
        "amount": selected_offer.total_amount,
        "type": "balance",
    }
]
passengers = [
    {
        "phone_number": phone_number,
        "email": email,
        "title": title,
        "gender": gender,
        "family_name": family_name,
        "given_name": given_name,
        "born_on": dob,
        "id": offer_request.passengers[0].id,
    }
]

order = await (
    client.orders.create()
    .payments(payments)
    .passengers(passengers)
    .selected_offers([selected_offer.id])
    .execute()
)

print("\n🎉 Flight booked. Congrats! You can start packing your (duffel?) bags")
print(f"Booking reference: {order.booking_reference}")
nlopes commented 1 year ago

Hey @cglacet,

Thank you for the suggestion. I think a lot of the criticism is valid. We won't be taking this right this moment but definitely something we'll consider.

cglacet commented 1 year ago

Hi @nlopes, don't hesitate to tag me here in case you need help. Depending on the timing I could help you find a good solution to this.