-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[MetricsAdvisor] Updated DataFeedSource constructors #22427
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,14 +17,37 @@ public class SqlServerDataFeedSource : DataFeedSource | |
| private string _connectionString; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="SqlServerDataFeedSource"/> class. | ||
| /// Initializes a new instance of the <see cref="SqlServerDataFeedSource"/> class. This constructor does not | ||
| /// set a <see cref="ConnectionString"/>, so you must assign an <see cref="AuthenticationType"/> to the | ||
| /// <see cref="Authentication"/> property. Currently, only the <see cref="AuthenticationType.SqlConnectionString"/> | ||
| /// authentication is supported without a connection string. If you intend to use another type of authentication, | ||
| /// see <see cref="SqlServerDataFeedSource(string, string)"/>. | ||
| /// </summary> | ||
| /// <param name="query">The query to retrieve the data to be ingested.</param> | ||
| /// <exception cref="ArgumentNullException"><paramref name="query"/> is null.</exception> | ||
| /// <exception cref="ArgumentException"><paramref name="query"/> is empty.</exception> | ||
| public SqlServerDataFeedSource(string query) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do u want to add a test for it? I think as is fine, but just in case u want to check query empty and the exception and etc...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests for new ctors. |
||
| : base(DataFeedSourceKind.SqlServer) | ||
| { | ||
| Argument.AssertNotNullOrEmpty(query, nameof(query)); | ||
|
|
||
| Query = query; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="SqlServerDataFeedSource"/> class. This constructor requires a | ||
| /// <paramref name="connectionString"/> and is intended to be used with the authentication types | ||
| /// <see cref="AuthenticationType.Basic"/> (default), <see cref="AuthenticationType.ManagedIdentity"/>, | ||
| /// <see cref="AuthenticationType.ServicePrincipal"/>, or <see cref="AuthenticationType.ServicePrincipalInKeyVault"/>. | ||
| /// If you intend to use <see cref="AuthenticationType.SqlConnectionString"/> authentication, see | ||
| /// <see cref="SqlServerDataFeedSource(string)"/>. | ||
| /// </summary> | ||
| /// <param name="connectionString">The connection string.</param> | ||
| /// <param name="query">The query to retrieve the data to be ingested.</param> | ||
| /// <exception cref="ArgumentNullException"><paramref name="connectionString"/> or <paramref name="query"/> is null.</exception> | ||
| /// <exception cref="ArgumentException"><paramref name="connectionString"/> or <paramref name="query"/> is empty.</exception> | ||
| public SqlServerDataFeedSource(string connectionString, string query) | ||
| : base(DataFeedSourceKind.SqlServer) | ||
| : this(query) | ||
| { | ||
| Argument.AssertNotNullOrEmpty(connectionString, nameof(connectionString)); | ||
| Argument.AssertNotNullOrEmpty(query, nameof(query)); | ||
|
|
||
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.
Not actually supported by the service. Other properties are required now. We'll possibly add this constructor back again as new types of credentials are added.