V4 alpha #188
Merged
V4 alpha #188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stripped back version of the API to incorporate the model name changes from V3 - Naming has been kept as close as possible to NodeSDK but there are some cases in which class name clashed so they were renamed
refactored Httpclient to be able to accept a preconfigured Httpclient refactored QueryParamaterUtil so that a callback url casing is respected - addressing github issues - #128
Refactored the AbstractRestClient so that it can either accept a HttpClient or a IHttpClientFactory. if a HttpClient is passed in it is stored in the parameter ExternalHttpClient. During a Rest call the methods will checker wheter a HttpClient or a IHttpClientFactory is present then assign or create HttpClient that will make the rest call, this allows for a timeout to be set at anytime. this will address both issues - Timeout -#127 Socket Exhaustion - #148
Added get and post request method and there tests
added the stubs for - Onprem, Prerecorded & Manage Methods
methods for ManageClient and OnpremClient created along with summary comments for the classes and methods
links to the equivalent files in the node sdk for what needs to be considered in creating this sdk
initial prerecorded client set, Note a difference from V3. instead of taking a FileSource in the TranscibeFile - there a two versions of this method,-- one that will accept a byte array(equivalent to a node buffer) one that will accept a stream(equivalent to node readable) Callback method have a return type as a Request id is returned no unit test done yet for any of the specific clients that derive from AbstractRestClient
correct call take either a byte[] or stream parameter so that there create streamcontent and not string content
rename the client method to have Async at the end of there name. all rest client now have 2 constructors, the default which does not take a DeepgramClientOptions and one that does. setting the proxy for HttpClient need to be done at time of creation. so doing this internally would mean managing the lifecycle of the HttpClient and would bloat the size of the sdk. It is much more practical to get the client to set the proxy when adding the HttpClient to services(readme update to show how this is done). Since it is most likely that when a proxy is needed it will be made for a named client. so internally when the create client method is called it needs to know the name of the named client, so this this has been added to the DeepgramClientOptions.
changed parameter order to so they make more sense
refactor how the httpclient options are passed to and processed by the sdk. they are now passed during service registration(needed to be done here for setting any proxy that needs to be used) Update the readme to reflect most of the changes added comments to public facing models
removal of a project that was used to test the servicecollection generation -- not needed
uncommeted the check for timeout - Note -- timeout can now be set as part of the deepgramclientoptions && by calling a method of the AbstractRestClient the timeout checks before the calls are there to allow for the flexibility to change timeout before making a call
deepgram options back to the constructor as the Custom headers and baseurl might be different for each client depending how a onprem is set up. Proxy for httpclient need to be set on the HttpMessageHandler which needs to be passed to the Httpclient on creation, so this need to be handle in the servicescollection
throwing System.PlatformNotSupportedException
update package references to use the latest minor build for a give major build updated test to reflect nunit code chages
refactored out duplicate test code to a SetUp attributed methods in the relevant classes. introduce FluentAssertions into test project. Make it easier to read the Assert Statements
cleaned up the library and language version to remove the Hash being added to the relevant versions
added redundencies in for people not wanting to/ or dont have access to servicecollection(in pre exisiting apps upgrading to this version of the sdk) use Dependency Injection such as if they just want to create a simple console app demo and new up a client added checks to see if there is a predefined client passed in - if there is then the sdk will assume the base address is in the correct format - the reason being that if it is a onPrem there is no way to know what there server address is reorganise tests so reading them makes more sense move the default baseaddress from DeepgramClientOptions to HttpClientExtensions added a check for a timeout
TopicGroup was not deseiralising when topic detection use on prerecorded client property was set to TopicGroup instead of List<TopicGroup>
Change service provision
Added the extra Property on the LiveSchema and the PrerecordedSchema
Added extra property
Sentiment added for the PrerecordedClient method Responses and required types create. However not add and implemented for the LiveClient.
Sentimentation partial complete
In trying to make the SDK work with or without Dependency injection as creating a over complicated implementation. If a User wants to use Dependency injection the can easily wrap the client the need and injected that themselves. Refactored HttpClient Creation to use a internal service collection and IHttpClientFactory Timeout needs to able to be adjusted inbetween method calls as if a user is not using a callback method on the prerecoreded client the HttpTimeout was being tripped quickly this becomes apparent when you run longer input files and increase the number of Schema options
Changed live client to be only emitting one event - a live response event, the user can query the value passed to the event to decided on how the want to deal with it.
Remove depedency injection
Cleared up models
move model to the architecture suggest in #159 Made it possible to add the APiVersion as part of the deepgram options instead of it being constant on v1
reorganised the models to fit architecture approach
added cancellation token to all calls to be able to cancel requests
Cancellation tokens added
davidvonthenen
previously approved these changes
Jan 31, 2024
davidvonthenen
approved these changes
Jan 31, 2024
This was referenced Feb 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pull request to move the v4-Alpha to main