dylanshine / openai-kit

A community Swift package used to interact with the OpenAI API
https://platform.openai.com/docs/api-reference
MIT License
692 stars 107 forks source link

Add URLSession support to Client #47

Closed ftp27 closed 1 year ago

ftp27 commented 1 year ago

I've got an error which is mentioned in issue #44. So I'm wondering if we can use URLSession to send requests.

dylanshine commented 1 year ago

@ftp27 This is great! Thank you for your contribution 👍 I think we're going to need to modify this implementation in such a way that would support Linux as well, you'll need to add checks to only expose URLSession apis in the appropriate environments

ftp27 commented 1 year ago

@dylanshine Thank you for the review. I'm really not sure I fixed the part with Linux. I don't have Linux to check it, hope it'll work.

dylanshine commented 1 year ago

@ftp27 The easiest thing here would probably be to wrap the Linux code with "#if os(Linux)" and define the macOS code in the else

ftp27 commented 1 year ago

@dylanshine Yep. I've done it in https://github.com/dylanshine/openai-kit/pull/47/commits/cfe369095a314a87d6d365d713c9f31a55232aa2

ftp27 commented 1 year ago

@dylanshine Is there anything else I can do?

dylanshine commented 1 year ago

@ftp27 Sorry, was out of town. You're on the right track, you just need to wrap the inverse as macOS compatible code will not run on Linux, however Linux code will run on macOS.

ftp27 commented 1 year ago

@dylanshine I tried to build a small executable app in docker. So now I see. It seems like Linux doesn't have async methods for URLSession (https://github.com/apple/swift-corelibs-foundation/issues/3205). Shell we just cut the feature off for Linux or reimplement async methods?

dylanshine commented 1 year ago

@ftp27 I think you can get this to work by just checking for macOS explicitly. On mac, you allow users to consume both the HTTPClient and URLSession APIs, and on Linux just the HTTPClient

ftp27 commented 1 year ago

@dylanshine I removed the URLSession code for Linux. Now it builds on Linux at least