Adding SqlPackage Service for supporting Sqlpackage command generation changes#2556
Adding SqlPackage Service for supporting Sqlpackage command generation changes#2556
Conversation
|
As part of updating the dependencies in Packages.props we require that any PRs opened also verify that Please respond to this comment verifying that you've done the appropriate validation (or explain why it's not necessary) before merging in the PR
|
… nuget publicly available
There was a problem hiding this comment.
Working on uploading the Sqlpackage Core nuget to nuget.org, uploaded temporarily for the validation checks
| <dependency id="Azure.Identity" version="1.12.0" /> | ||
| <dependency id="Microsoft.Data.SqlClient" version="5.1.6" /> | ||
| <dependency id="Azure.Identity" version="1.14.2" /> | ||
| <dependency id="Microsoft.Data.SqlClient" version="6.1.3" /> |
There was a problem hiding this comment.
This is a pretty significant bump in sqlclient. We will have to make sure if everything is still working.
There was a problem hiding this comment.
we are not bumping the MDS with this release so, reverted the changes and fixed the issue with the Nuget that causes the vbump requirement
Packages.props
Outdated
| <PackageReference Update="Azure.Identity" Version="1.12.0" /> | ||
| <PackageReference Update="Microsoft.Data.SqlClient" Version="5.1.7" /> | ||
| <PackageReference Update="Azure.Identity" Version="1.14.2" /> | ||
| <PackageReference Update="Microsoft.Data.SqlClient" Version="6.1.3" /> |
There was a problem hiding this comment.
I thought we decided yesterday that we don't need to update MDS for this release. is it required for your work?
There was a problem hiding this comment.
Since the New Nuget's sqlpackage.dll build against latest dacfx, it is expecting the MDS version bump and few supported packages upgrade. But I'll verify again if the version upgrades can be bypassed with the nuget usage.
There was a problem hiding this comment.
you nuget should have dependency on dacfx 170.2. that's what we discussed in the meeting.
| await requestContext.SendResult(new SqlPackageCommandResult | ||
| { | ||
| Success = false, | ||
| ErrorMessage = "Invalid request parameters" |
| /// <summary> | ||
| /// Enable diagnostics logging | ||
| /// </summary> | ||
| public bool Diagnostics { get; set; } |
There was a problem hiding this comment.
you cannot inherit from CommandLineArguments so you don't duplicate all the properties?
There was a problem hiding this comment.
great point! actually we can avoid repetition of this by doing inheritance, since i have started with few arguments before i made the commandlineArgs public, I missed this one. Thanks.
packages/Microsoft.SqlTools.ManagedBatchParser/Microsoft.SqlTools.ManagedBatchParser.nuspec
Outdated
Show resolved
Hide resolved
| <group targetFramework="net8.0"> | ||
| <dependency id="Azure.Identity" version="1.12.0" /> | ||
| <dependency id="Microsoft.Data.SqlClient" version="5.1.7" /> | ||
| <dependency id="Microsoft.Data.SqlClient" version="5.1.6" /> |
There was a problem hiding this comment.
Batchparser is at 5.1.6 currenlty, should be align with MDS version with props file though. we are not making any changes to that in this PR.
| @@ -1,5 +1,5 @@ | |||
| // WARNING: | |||
| // This file was generated by the Microsoft SQL Server String Resource Tool 9.0.0.0 | |||
| // This file was generated by the Microsoft SQL Server String Resource Tool 10.0.0.0 | |||
There was a problem hiding this comment.
Do you know why we are seeing these changes?
There was a problem hiding this comment.
I ran the localization command to generate the loc strings, and it produced these updates to the files.
Pull Request Template – SQL Tools Service
Description
This PR provides the Service layer that bridges VSCode and the SqlPackage API by transforming client-side deployment configurations into SqlPackage CLI command strings, handling contract conversion and option normalization.
Below are the complete changes that have made to achieve this task with sqlpackage service.
Code Changes Checklist
dotnet test)Reviewers: Please read our reviewer guidelines