Remove manual Content-Length header from v1/v2 Utilities #388
Conversation
This file contains hidden or 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
WalkthroughRemoved manual "Content-Length" header setting from CreateStreamPayload(Stream body) in Utilities for v1 and v2, leaving StreamContent creation unchanged and relying on default handling for content length. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The following sample code was used to verify the change with a live WebSocket stream: // Copyright 2024 Deepgram .NET SDK contributors. All Rights Reserved.
// Use of this source code is governed by a MIT license that can be found in the LICENSE file.
// SPDX-License-Identifier: MIT
using Deepgram.Models.Listen.v2.WebSocket;
namespace SampleApp
{
class Program
{
static async Task Main(string[] args)
{
try
{
// Initialize Library with default logging
Library.Initialize();
// use the client factory with a API Key set with the "DEEPGRAM_API_KEY" environment variable
var liveClient = new ListenWebSocketClient();
// Subscribe to the EventResponseReceived event
await liveClient.Subscribe(new EventHandler<ResultResponse>((sender, e) =>
{
if (e.Channel.Alternatives[0].Transcript == "")
{
return;
}
Console.WriteLine($"Speaker: {e.Channel.Alternatives[0].Transcript}");
}));
// Start the connection
var liveSchema = new LiveSchema()
{
Model = "nova-2",
Punctuate = true,
SmartFormat = true,
};
bool bConnected = await liveClient.Connect(liveSchema);
if (!bConnected)
{
Console.WriteLine("Failed to connect to the server");
return;
}
// get the webcast data... this is a blocking operation
try
{
var url = "https://drive.google.com/uc?export=download&id=1GMyga9gpiv1G_ulu4xq1MWg3-WAHU-sf";
using (HttpClient client = new HttpClient())
{
var response = await client.GetAsync(url, HttpCompletionOption.ResponseHeadersRead);
Console.WriteLine("Response Headers:");
foreach (var header in response.Content.Headers)
{
Console.WriteLine($" {header.Key}: {string.Join(", ", header.Value)}");
}
using (Stream receiveStream = await response.Content.ReadAsStreamAsync())
{
while (liveClient.IsConnected())
{
byte[] buffer = new byte[2048];
await receiveStream.ReadAsync(buffer, 0, buffer.Length);
liveClient.Send(buffer);
}
}
}
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
// Stop the connection
await liveClient.Stop();
// Teardown Library
Library.Terminate();
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
}
}
}Observed Behavior: Before this PR: After this PR: Impact:
|
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.
Proposed changes
This PR removes the explicit Content-Length header from Utilities.cs in both the v1 and v2 abstractions.
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
No additional tests were added because this is a minimal change.
HttpClientautomatically handlesContent-LengthforStreamContent, so removing this header does not change observable behavior.All existing tests pass, confirming the SDK continues to function as expected.
Summary by CodeRabbit