Extract OpenAI interface
Closed this issue · 3 comments
Define an interface (perhaps with named OpenAI). The interface can then be used for vanilla OpenAI implementation, Azure OpenAI implementation and other possible implementations in the future.
I see where you are coming from, but that might be a bit too "abstracted". The base implementation with overridable functions is probably the best approach for readability and simplicity.
A change like this would also be API breaking.
As you pointed out, there will probably be increased number of OpenAI "implmentations". For example OpenAIV2, OpenAIV3, AzureOpenV2 etc... How are we supposed to work with these classes? Use them directly? That's not why OOP was invented.
As stated in SOLID principles by Robert C. Martin:
Liskov Substitution Principle (LSP): Subtypes must be substitutable for their base types. This means that objects of derived classes should be able to replace objects of the base class without affecting the correctness of the program. => OpenAI is NOT a base type for AzureOpenAI, because it works internally differently. If we had an abstracted OpenAI type, that would be different, of course.
Dependency Inversion Principle (DIP): High-level modules should not depend on low-level modules. Both should depend on abstractions. This principle promotes the use of interfaces and abstract classes to decouple components and create flexible systems. => this is exactly what frameworks like Spring adhere to and why they are so successful. By not providing an interface, we directly go against Spring's philosophy and making it hard to use in such frameworks (btw I already faced such issue in my project with this library).
btw without providing a really abstract interface, you are making the library harder to extend and therefore less likely to be used. Because other developers have no way of "interfacing" with the functionality of this library.
In few months there will be a lot of new classes and no programmer will be able to understand which should be used when unless they read the documentation and the code. That is a bad library in action, in my personal opinion.
I don't believe there will be OpenAI v2, but a v2/whisper endpoint seems possible (maybe likely?). In this case, we wouldn't be reimplementing an entirely new OpenAI class, instead creating a new method to interact with that 1 endpoint.
But for devils advocate, let's assume OpenAI put the v1 endpoint so that they can rerelease the company under v2 with completely different endpoints and functionality. In this case, trying to add these methods into an interface breaks existing programs. Instead, a default implementation (OpenAI) or a completely separate class is best.
OpenAI being the base class, in context of both LSP and DIP makes sense. I agree with you 100% that Azure/OpenAI work internally different. I look at it as if java collections had the LinkedList extends ArrayList... how horrible that would be! But also consider that we do not interact with the internals of AzureOpenAI or OpenAI. We interact with their API which is more or less the same. So if you look at it as the AzureOpenAI endpoints extending the OpenAI endpoints, both LSP and DIP are followed.
btw without providing a really abstract interface, you are making the library harder to extend and therefore less likely to be used. Because other developers have no way of "interfacing" with the functionality of this library.
I'm with you here. I am used to explicitly marking methods as final in java, and I don't often think about how in kotlin. Users should be able to override these API methods if they wanted to add custom logic. I definitely should mark most of these methods as open.
Just to sound professional I'll cite my favorite CS principle, DRY. Having OpenAI as the base type allows default implementations for the methods and helps repeating code between OpenAI and AzureOpenAI.
btw I already faced such issue in my project with this library
I'm not exactly sure which issue you have faced... more context might be needed to justify changes.