sqlpackage path argument, dotnet tool sqlpackage #221

dzsquared · 2024-05-13T19:21:20Z

3 changes lumped together in this PR (I let it get away from me):

for dacpac and sqlproj action there's an optional action parameter sqlpackage-path which overrides any sqlpackage location detection by the action except to check that it exists. (fixes Support SqlPackage.exe at a specific path #211)
dotnet tool location for a global install of sqlpackage is checked
the full semantic version is parsed for version comparison, instead of just 150 vs 160, which was leading to the wrong version being picked (162.1 vs 162.2 would default to one based on who installed instead of the actual version)

also fixes #180

future PR needed to remove SSMS-related logic as it hasn't installed sqlpackage.exe for quite some time

…kwier/dotnet-tool

src/AzureSqlActionHelper.ts

github-actions · 2024-06-04T00:21:42Z

This PR is idle because it has been open for 14 days with no activity.

…tnet-tool

…kwier/dotnet-tool

__tests__/AzureSqlActionHelper.test.ts

src/AzureSqlActionHelper.ts

chlafreniere · 2024-06-27T22:52:06Z

src/AzureSqlActionHelper.ts

+        core.debug('Getting location of SqlPackage');
+
+        let sqlPackagePathInstalledWithDotnetTool = await this._getSqlPackageExeInstalledDotnetTool();
+        core.debug(`SqlPackage (installed with dotnet tool) found at location: ${sqlPackagePathInstalledWithDotnetTool[0]}, version ${sqlPackagePathInstalledWithDotnetTool[1]}`);


I'm not really following this debug logic, and this could be confusing to customers - shouldn't we only say that sqlpackage was found if it was actually found (and instead, output a different message if it weren't found)?

chlafreniere · 2024-06-27T22:57:59Z

src/AzureSqlActionHelper.ts

+        core.debug('Getting location of SqlPackage');
+
+        let sqlPackagePathInstalledWithDotnetTool = await this._getSqlPackageExeInstalledDotnetTool();
+        core.debug(`SqlPackage (installed with dotnet tool) found at location: ${sqlPackagePathInstalledWithDotnetTool[0]}, version ${sqlPackagePathInstalledWithDotnetTool[1]}`);


This is me being picky, but I much prefer an interface to be returned here instead of an array of a string + semver to prevent array lookups in various places

chlafreniere · 2024-06-27T23:10:25Z

src/AzureSqlActionHelper.ts

+        let globalDotnetToolsPath = path.join(process.env['USERPROFILE'] as string, '.dotnet', 'tools');
+        let sqlPackagePath = path.join(globalDotnetToolsPath, 'SqlPackage.exe');
+        if (fs.existsSync(sqlPackagePath)) {
+            core.debug(`SqlPackage (installed with dotnet tool) found at location: ${sqlPackagePath}`);


Are there going to be double debug statements (due to debug statements also existing in _getSqlPackageExecutablePath)

yep, that would have been too noisy - thanks - cleaned up and moved all positive case debug messages to the specific functions

src/AzureSqlActionHelper.ts

…to drskwier/dotnet-tool

chlafreniere

Looks great, thanks so much for being receptive to comments/suggestions 😄!

dzsquared added 4 commits May 13, 2024 12:14

irritating spelling error

d94306d

sqlpackage path optional, dotnet tool

dcc5313

minor test coverage

abd7cdb

main.js build for ci tests

8c5c18f

dzsquared requested review from zijchen, yorek and chlafreniere as code owners May 13, 2024 19:21

dzsquared temporarily deployed to Automation test May 13, 2024 19:21 — with GitHub Actions Inactive

dzsquared marked this pull request as draft May 13, 2024 19:33

Merge branch 'master' of https://github.com/Azure/sql-action into drs…

6836429

…kwier/dotnet-tool

dzsquared temporarily deployed to Automation test May 14, 2024 20:27 — with GitHub Actions Inactive

zijchen reviewed May 20, 2024

View reviewed changes

src/AzureSqlActionHelper.ts Outdated Show resolved Hide resolved

src/AzureSqlActionHelper.ts Outdated Show resolved Hide resolved

src/AzureSqlActionHelper.ts Show resolved Hide resolved

src/AzureSqlActionHelper.ts Outdated Show resolved Hide resolved

github-actions bot added the idle Inactive for 14 days label Jun 4, 2024

Merge branch 'master' of github.com:Azure/sql-action into drskwier/do…

01a8921

…tnet-tool

dzsquared temporarily deployed to Automation test June 13, 2024 18:52 — with GitHub Actions Inactive

code cleanup and tests started

6c10417

dzsquared temporarily deployed to Automation test June 14, 2024 16:33 — with GitHub Actions Inactive

working around the linux CI failures

40625b6

dzsquared temporarily deployed to Automation test June 14, 2024 18:12 — with GitHub Actions Inactive

Fix package-lock.json and rebuild

ca506f3

zijchen temporarily deployed to Automation test June 14, 2024 19:22 — with GitHub Actions Inactive

fix on no path test

9630ea0

dzsquared temporarily deployed to Automation test June 14, 2024 23:00 — with GitHub Actions Inactive

dzsquared temporarily deployed to Automation test June 16, 2024 20:15 — with GitHub Actions Inactive

dzsquared marked this pull request as ready for review June 16, 2024 20:18

dzsquared requested a review from zijchen June 17, 2024 18:03

zijchen previously approved these changes Jun 19, 2024

View reviewed changes

dzsquared and others added 2 commits June 21, 2024 11:39

Merge branch 'master' of github.com:Azure/sql-action into drskwier/do…

78a9bcf

…tnet-tool

Merge branch 'master' of https://github.com/Azure/sql-action into drs…

f12eda2

…kwier/dotnet-tool

zijchen dismissed their stale review via f12eda2 June 27, 2024 22:39

zijchen temporarily deployed to Automation test June 27, 2024 22:39 — with GitHub Actions Inactive

chlafreniere reviewed Jun 27, 2024

View reviewed changes

dzsquared added 2 commits June 28, 2024 10:03

Merge branch 'drskwier/dotnet-tool' of github.com:Azure/sql-action in…

9997939

…to drskwier/dotnet-tool

sqlpackage info to interface, cleanup debug messages

062418e

dzsquared had a problem deploying to Automation test June 28, 2024 17:55 — with GitHub Actions Failure

dzsquared temporarily deployed to Automation test June 28, 2024 17:55 — with GitHub Actions Inactive

windows pipeline debugging

e47ed2f

dzsquared had a problem deploying to Automation test June 28, 2024 18:53 — with GitHub Actions Failure

dzsquared temporarily deployed to Automation test June 28, 2024 18:53 — with GitHub Actions Inactive

correcting not found logic

e6948cf

dzsquared temporarily deployed to Automation test June 28, 2024 19:07 — with GitHub Actions Inactive

remove diagnostic messages

da311e6

dzsquared temporarily deployed to Automation test June 28, 2024 19:20 — with GitHub Actions Inactive

dzsquared requested review from chlafreniere and zijchen June 28, 2024 19:32

chlafreniere approved these changes Jun 28, 2024

View reviewed changes

zijchen approved these changes Jun 28, 2024

View reviewed changes

dzsquared merged commit 83d6b65 into master Jun 28, 2024
6 checks passed

dzsquared deleted the drskwier/dotnet-tool branch June 28, 2024 21:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sqlpackage path argument, dotnet tool sqlpackage #221

sqlpackage path argument, dotnet tool sqlpackage #221

dzsquared commented May 13, 2024 •

edited

Loading

github-actions bot commented Jun 4, 2024

chlafreniere Jun 27, 2024

chlafreniere Jun 27, 2024

chlafreniere Jun 27, 2024

dzsquared Jun 28, 2024

chlafreniere left a comment

sqlpackage path argument, dotnet tool sqlpackage #221

sqlpackage path argument, dotnet tool sqlpackage #221

Conversation

dzsquared commented May 13, 2024 • edited Loading

github-actions bot commented Jun 4, 2024

chlafreniere Jun 27, 2024

Choose a reason for hiding this comment

chlafreniere Jun 27, 2024

Choose a reason for hiding this comment

chlafreniere Jun 27, 2024

Choose a reason for hiding this comment

dzsquared Jun 28, 2024

Choose a reason for hiding this comment

chlafreniere left a comment

Choose a reason for hiding this comment

dzsquared commented May 13, 2024 •

edited

Loading