-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor the listener to implement a decorator around openai.Completion.create #47
Conversation
ad1d504
to
866cfae
Compare
|
||
|
||
def start_openai_listener(): | ||
openai.Completion = CustomCompletion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's really neat! monkey patching 🐵
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the Completion
API has been marked as legacy now. Should we be patching the ChatCompletions
API instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrading to ChatCompletion is a refactor that would need its own ticket I believe
https://trello.com/c/rkkADWMv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the usage of the Completion
API intentional? Was there a reason it was implemented over other APIs?
I agree about the ticket!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not intentional, just took the example off the OpenAI docs
866cfae
to
3d78163
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not catch this earlier, but is there any reason we are using relative imports instead of absolute?
iirc relative imports are highly discouraged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my device absolute imports are calling the published module instead of my dev folder...
```sh | ||
pyenv install 3.11.3 | ||
poetry env use 3.11.3 | ||
poetry install | ||
``` | ||
|
||
2. Generate prisma types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
The ticket mentions a test to delete - does that need to be done? |
No description provided.