Skip to content
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

Do not introduce breaking changes in minor versions #1000

Closed
janousek opened this issue May 14, 2020 · 3 comments · Fixed by #1001
Closed

Do not introduce breaking changes in minor versions #1000

janousek opened this issue May 14, 2020 · 3 comments · Fixed by #1001
Labels
status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library

Comments

@janousek
Copy link

janousek commented May 14, 2020

Issue Summary

Version 9.15.0 contains undocumented breaking changes in SendGridCient. Now it is impossible to pass own configuration using SendGridClientOptions. Because of this it is impossible to set some of options avaiable in SendGridClientOptions (for example ReliabilitySettings).

And now I discovered that ReliabilitySettings is totaly ignored (it is hardcoded):

return new HttpClient(new RetryDelegatingHandler(new ReliabilitySettings()));

var retryHandler = new RetryDelegatingHandler(httpClientHandler, new ReliabilitySettings());

This is shame. Your developers should really think about changes that they make (merget to master branch) before you publish new version. You are breaking existing code.

@childish-sambino
Copy link
Contributor

So this is the old line:

return new HttpClient(new RetryDelegatingHandler(DefaultOptions.ReliabilitySettings));

It uses DefaultOptions which was defined here:

private static readonly SendGridClientOptions DefaultOptions = new SendGridClientOptions();

How were you able to modify the ReliabilitySettings of DefaultOptions? I don't see any accessors for DefaultOptions or its ReliabilitySettings (when looking at the code before the changes in question).

@childish-sambino childish-sambino added status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels May 14, 2020
@janousek
Copy link
Author

ReliabilitySettings gets broken in this commit:
d6c2349

@childish-sambino
Copy link
Contributor

Ahhh, perfect. Thanks! So it's been broken since 9.11.0 and now I understand why it was the way it was before I refactored things in #997. I tried hard to not break the interface, but turns out it was already borked. I'll work on a fix. Thanks for bringing this to our attention!

And just to be clear, you're basically wanting this constructor back so you can pass in your own SendGridClientOptions instance?

/// <summary>
/// Initializes a new instance of the <see cref="SendGridClient"/> class.
/// </summary>
/// <param name="options">A <see cref="SendGridClientOptions"/> instance that defines the configuration settings to use with the client </param>
/// <returns>Interface to the Twilio SendGrid REST API</returns>
public SendGridClient(SendGridClientOptions options)
: this(null, options)
{
}

@childish-sambino childish-sambino added status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library and removed status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library labels May 14, 2020
childish-sambino pushed a commit that referenced this issue May 14, 2020
… its reliability settings

Fixes #1000
Relates to #839

When the default client options were made static and the constructor was removed, there was no way to modify the reliability settings used by the retry handler. This fix adds back the constructor so clients can now be constructed with just the client options and the options will be used when creating a retry handled, if any.
childish-sambino pushed a commit that referenced this issue May 14, 2020
… its reliability settings

Fixes #1000
Relates to #839

When the default client options were made static and the constructor was removed, there was no way to modify the reliability settings used by the retry handler. This fix adds back the constructor so clients can now be constructed with just the client options and the options will be used when creating a retry handler, if any.
childish-sambino pushed a commit that referenced this issue May 14, 2020
…e its reliability settings

Fixes #1000
Relates to #839

When the default client options were made static and the constructors later removed, there was no way to modify the reliability settings used by the retry handler. This fix adds back the constructors so clients can now be constructed with just the client options and the options will be used when creating a retry handler, if any.
childish-sambino pushed a commit that referenced this issue May 14, 2020
…e its reliability settings

Fixes #1000
Relates to #839

When the default client options were made static and the constructors later removed, there was no way to modify the reliability settings used by the retry handler. This fix adds back the constructors so clients can now be constructed with just the client options and the options will be used when creating a retry handler, if any.
childish-sambino pushed a commit that referenced this issue May 19, 2020
…e its reliability settings (#1001)

Fixes #1000
Relates to #839

When the default client options were made static and the constructors later removed, there was no way to modify the reliability settings used by the retry handler. This fix adds back the constructors so clients can now be constructed with just the client options and the options will be used when creating a retry handler, if any.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library
Projects
None yet
2 participants