Introduce public preview SQL Database Agent .NET SDK #4340
Conversation
…o grab subscription id from env variable
|
@jaredmoo Initial pass at the SQL Database Agent .NET SDK PR is set up. |
jaredmoo
left a comment
jaredmoo
left a comment
There was a problem hiding this comment.
Please also bump version (latest published is 1.15.0 as you can see at https://www.nuget.org/packages/Microsoft.Azure.Management.Sql , so bump to 1.16.0) & update release notes in Microsoft.Azure.Management.Sql.csproj. And bump version in AssemblyInfo.cs.
| // TEST_CSM_ORGID_AUTHENTICATION environment variable in this format then: | ||
| // SubscriptionId={SubId};ServicePrincipal={clientId};ServicePrincipalSecret={clientSecret};AADTenant={tenantId};Environment={env};HttpRecorderMode=Record; | ||
| // The below string split and getting the {SubId} should work. | ||
| this._subscriptionId = Environment.GetEnvironmentVariables()["TEST_CSM_ORGID_AUTHENTICATION"].ToString().Split(';')[0].Split('=')[1]; |
There was a problem hiding this comment.
I think this won't work in test playback, but you can use sqlManagementClient.SubscriptionId instead.
There was a problem hiding this comment.
Won't need it since we can just use credential.Id and targetGroup.Id :)
| JobCredential credential = sqlClient.JobCredentials.CreateOrUpdate(resourceGroup.Name, server.Name, agent.Name, SqlManagementTestUtilities.DefaultLogin, new JobCredential | ||
| { | ||
| Username = "cloudsa", | ||
| Password = "Yukon900!" |
There was a problem hiding this comment.
You could use
Username = SqlManagementTestUtilities.DefaultLogin,
Password = SqlManagementTestUtilities.DefaultPassword,
There was a problem hiding this comment.
Never mind, you update below. Can you make this initial credential more fake looking, like just "a" and "b"
| { | ||
| ServerName = "s1", | ||
| Type = JobTargetType.SqlServer, | ||
| RefreshCredential = FormatCredentialId(resourceGroup.Name, server.Name, agent.Name, credential.Name), |
There was a problem hiding this comment.
You can just used credential.Id ;)
| }); | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
nit: excessive whitespace?
| { | ||
| Value = "SELECT 1" | ||
| }, | ||
| TargetGroup = FormatTargetGroupId(resourceGroup.Name, server.Name, agent.Name, targetGroup.Name) |
|
Note to SDK team: we also have another SQL feature #4327 coming in at the same time, we will make sure to coordinate between these 2 features. Most likely we can get them completed and published together in the next few days. |
… Updated tests based on feedback
| * ElasticPool.PerDatabaseSettings has replaced ElasticPool.DatabaseDtuMin and ElasticPool.DatabaseDtuMax. | ||
| - Database.MaxSizeBytes is now a long instead of string. | ||
| - LocationCapabilities tree has been changed in order to support capabilities of new vCore-based database and elastic pool editions. | ||
| - Adding support for SQL Database Agent |
There was a problem hiding this comment.
@jaredmoo Is this new features description too plain? Let me know if you'd like a more detailed one.
I was unsure how detailed to go for a release update.
There was a problem hiding this comment.
That is totally fine. Maybe also mention the type names (JobAgent, Job, JobStep, ....) so that it is obvious which types are related to DB agent?
dsgouda
left a comment
dsgouda
left a comment
There was a problem hiding this comment.
Please run msbuild build.proj /t:build /p:Scope=SDs\SqlManagement and commit any changes to the .props file
| Branch: master | ||
| Commit: d08c7f54a125819caaa8c5f553206d1adbae60f9 | ||
| GitHub user: johnpaulkee | ||
| Branch: jobsSdk |
There was a problem hiding this comment.
Code MUST be generated from a spec in the Azure master branch
There was a problem hiding this comment.
We did this in order to isolate this change from other APIs added to readme.md at the same time.
It was just this change to readme: https://github.com/johnpaulkee/azure-rest-api-specs/commit/992e3dd8dfe722697d045458f827e9d554ef6ebb
There was a problem hiding this comment.
I'm not sure this is a good strategy. Either create and use a tag in the config file to scope the files you'd like to generate the code for or we could create a whitelisted branch in the repo to coordinate the work before merging with psSdkJson6
@shahabhijeet can you lend some thoughts too?
…s ran msbuild build.proj /t:build /p:Scope=SDs\SqlManagement
|
@dsgouda I've regenerated from azure master. Note that there will be some included generated SDKs for DatabaseVulnerabilityAssessment, VulnerabilityAssessment, and ShortTermRetentionPolicies that will be added too due to this. FYI @yaakoviyun @dealaus @jaredmoo I heard from @dsgoudsa that it's fine to add the other SDKs as part of this PR since we can add tests to them later. let me know your thoughts on that. |
|
It's fine to include the other features, but the risk to us is that @yaakoviyun & @dealaus will need to add tests before we can release. |
|
This PR was merged into #4351 and can be closed, |
3 participants
Description
This is a preview SDK for the new SQL Database Agent introduced in the 2017-03-01 preview API version.
Added the corresponding tests. Pull request for Swagger: Azure/azure-rest-api-specs#2909
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK.