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

feat: Add support for AIProxy in Swift #120

Merged

Conversation

lzell
Copy link
Contributor

@lzell lzell commented Jul 30, 2024

WeatherApiResponse has a public static method fetch(url:session:) that customers use to interact with the Open-Meteo API. This patch opens a second public static method, fetch(request:session:) that takes a URLRequest as the first argument.

Why?
By passing in a URLRequest, the developer is able to customize headers, which is useful if the request is first routing through a proxy. There is an AIProxy customer that would like to use your commercial API. In order for your service to be compatible with AIProxy, the end developer needs to be able to specify request headers (e.g. a DeviceCheck header, and a header that contains part of a split-key encryption scheme). These headers allow AIProxy to defend against actors that would try to steal a secret API key, or abuse a service endpoint.

I have found that the least invasive way to modify your SDK is to open this new public method. I'll open this PR as a draft to address any concerns that you may have, and I'm happy to answer questions!

@patrick-zippenfenig patrick-zippenfenig changed the title Draft: Add support for AIProxy feat: Add support for AIProxy in Swift Jul 31, 2024
@patrick-zippenfenig
Copy link
Member

Looks good. Do you use any other libraries like async-http-client? Now with better async support in Swift, I could add support for AsyncSequences as well.

@lzell
Copy link
Contributor Author

lzell commented Aug 1, 2024

Thanks for approving! I don't personally use async-http-client, but it looks nice.

For my needs, I find the async/await versions of Foundation's URLSessionDataTask to be sufficient, since the requests are originating from an app and they are relatively infrequent. If I were doing some heavier lifting, I'd consider dropping in that client (the nonblocking I/O aspects look most handy for dealing with lots of requests with a small memory footprint).

For my purposes, users of AIProxy are making a small number of requests from an app. The requests route from the app -> aiproxy -> the final destination, in this case Open-Meteo! AIProxy sits in the middle and applies DeviceCheck, banning, and rate limiting functionality so that the app developer gets a predictable bill. Hope that helps! Happy to chat more too, on here or twitter or [email protected]

In some cases, I try to bake AIProxy support directly into the client sdk. In this case, since you expose almost everything I need already, I think just opening this new interface is enough for me. If you are curious what a deeper integration looks like, you can see an example here: jamesrochabrun/SwiftAnthropic#26

I'm also thinking of extracting the common parts of that PR into a little lib on its own so that folks can add AIProxy support to their own SDKs with ease

@lzell lzell marked this pull request as ready for review August 1, 2024 18:40
@lzell lzell force-pushed the pass-urlrequest-into-public-fetch branch from 26666b0 to 906c6f0 Compare August 1, 2024 22:55
@lzell
Copy link
Contributor Author

lzell commented Aug 1, 2024

@patrick-zippenfenig FYI I force-pushed a new commit that uses my GPG signature to satisfy your workflow

@lzell
Copy link
Contributor Author

lzell commented Aug 2, 2024

Hi @patrick-zippenfenig, do you mind taking another look? I'm still seeing this:

Screenshot 2024-08-02 at 08 29 14

@patrick-zippenfenig patrick-zippenfenig merged commit 6d92430 into open-meteo:main Aug 2, 2024
2 checks passed
Copy link

github-actions bot commented Aug 2, 2024

🎉 This issue has been resolved in version 1.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lzell
Copy link
Contributor Author

lzell commented Aug 2, 2024

Thank you @patrick-zippenfenig !

@patrick-zippenfenig
Copy link
Member

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants