DbUp / dbup-sqlserver

Microsoft SQL Server provider for DbUp

MIT License

4 stars 3 forks source link

Update Azure SQL Authentication to use Azure.Identity #14

Open evochriso opened 4 months ago

evochriso commented 4 months ago

Checklist

[X] I have read the Contributing Guide
[X] I have checked to ensure this does not introduce an unintended breaking changes
[X] I have considered appropriate testing for my change

Description

Replace deprecated package Microsoft.Azure.Services.AppAuthentication with Azure.Identity Resolves #12

droyad commented 4 months ago

Thanks I'll pull this in when we drop support System.Data.SqlClient (a breaking change).

Bartleby2718 commented 4 months ago

@droyad If you don't mind educating a fellow engineer, would you be willing to explain why it'd be better to wait for another major version bump?

My understanding is that:

The public API hasn't changed, so this change in and of itself is not a breaking change.
The new dependency Azure.Identity doesn't have major version 2, so I don't think consumers will be negatively affected by the new dependency.
The issue was first reported in July 2022, so I'm sure there are people waiting for this.

(Hope I didn't miss anything obvious.)

evochriso commented 4 months ago

Thanks I'll pull this in when we drop support System.Data.SqlClient (a breaking change).

@droyad this update shouldn't be a breaking change since it doesn't change behavior or API surface, just swaps a deprecated dependency for its replacement, and has no dependency on System.Data.SqlClient. Perhaps a minor version bump instead of wait for the next major release? As mentioned by @Bartleby2718 - the usage of the deprecated package is causing dbup-sqlserver to be flagged by security scanners and a higher priority issue than removing the System.Data.SqlClient package which is not yet deprecated (just needs version bump to resolve security vulnerability).