Skip to content

Conversation

@johnd0e
Copy link
Contributor

@johnd0e johnd0e commented Apr 4, 2024

This is rather a question.
Is there a reason to unconditionally alter openai defaults?

@johnd0e
Copy link
Contributor Author

johnd0e commented Apr 5, 2024

BTW, it may also make sense to remove defaults for models parameter, instead of #1.

@leafo
Copy link
Owner

leafo commented Apr 17, 2024

I don't think there was a particular reason, probably just left over when testing.

cjson.encode payload

headers = {
"Host": parse_url(@api_base).host
Copy link
Owner

Choose a reason for hiding this comment

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

Host header is part of HTTP spec, not related to any of the APIs. As this library tries to support many environments/clients, in my experience I've had some issues with Lua http clients not correctly setting host header based on the request URL. For that reason I would not remove it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Firstly, in my humble opinion, it appears that we are trying to solve a problem that does not yet exist.

Having tested the LuaSocket with numerous different services without explicitly setting the Host in the library code, I didn’t encounter any problems. Of course, there is always the possibility someone, someday, might encounter an issue in a different environment.

In such an event, we would still have three options:

  1. Request a proper fix in the appropriate location (i.e. the custom Lua HTTP client).
  2. Instead of waiting for it to be fixed, set the header in the user code with solution Support additional headers #4.
  3. As a last resort, we can always modify the Lua-OpenAI code itself, preferably not prematurely.

However, the final decision is ultimately up to you and I will withdraw the last commit if you insist.

@johnd0e johnd0e marked this pull request as ready for review April 17, 2024 15:53
johnd0e added 4 commits April 18, 2024 00:12
The models come and go,  it is the client's responsibility to stay updated.

P.S.
We could ensure that the model is specified, but what's the point?
The server returns an error without needing assistance from the library."
Ref:
- https://platform.openai.com/docs/api-reference/making-requests

Instead, we should implement the ability to include arbitrary headers
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