-
Notifications
You must be signed in to change notification settings - Fork 7
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
add CancellationToken #13
Conversation
@@ -31,7 +32,7 @@ public PlaintextGraphiteFormatter(ushort port):this() | |||
public ushort Port { get; } | |||
|
|||
/// <inheritdoc/> | |||
public async Task WriteAsync(Stream stream, ICollection<Datapoint> datapoints) | |||
public async Task WriteAsync(Stream stream, ICollection<Datapoint> datapoints, CancellationToken cancellationToken = default(CancellationToken)) | |||
{ | |||
using (var writer = new StreamWriter(stream) {NewLine = "\n"}) | |||
{ |
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.
Konsequenterweise würde ich hier vor den WriteAsync- und Flush-Aufrufen am StreamWriter das CancellationToken prüfen. Oder soll das explizit ignoriert werden?
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.
StreamWriter.WriteAsync() und StreamWriter.FlushAsync() haben keinen cancellationToken parameter.
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.
Ich weiß. Deshalb müsste man das selbst prüfen, meinte ich.
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.
fixed
Graphite/GraphiteClient.cs
Outdated
/// Send a list of datapoints in up to <see cref="BatchSize"/> batches | ||
/// </summary> | ||
/// <param name="datapoints"></param> | ||
/// <param name="cancellationToken">The token to monitor for cancellation requests. The default value is <see cref="CancellationToken.None"/>.</param> |
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.
Comment does not match method definition (no default value)
} | ||
} | ||
|
||
private async Task SendInternalAsync(ICollection<Datapoint> datapoints) | ||
private async Task SendInternalAsync(ICollection<Datapoint> datapoints, CancellationToken cancellationToken) | ||
{ | ||
using (var client = new TcpClient(AddressFamily.InterNetworkV6)) | ||
{ | ||
client.Client.DualMode = true; | ||
await client.ConnectAsync(Host, Formatter.Port).ConfigureAwait(false); |
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.
consider another ThrowIfCancellationRequested here?
check cancellationToken in SendInternalAsync
add CancellationToken to all async methods