Skip to content
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

11.2.0 fails to deploy in OSGi environment without msal4j #1892

Closed
MrEasy opened this issue Aug 9, 2022 · 3 comments · Fixed by #1893
Closed

11.2.0 fails to deploy in OSGi environment without msal4j #1892

MrEasy opened this issue Aug 9, 2022 · 3 comments · Fixed by #1893
Milestone

Comments

@MrEasy
Copy link
Contributor

MrEasy commented Aug 9, 2022

Driver version

11.2.0

SQL Server version

Microsoft SQL Server 2019 (RTM-GDR) (KB5014356) - 15.0.2095.3 (X64) Apr 29 2022 18:00:13 Copyright (C) 2019 Microsoft Corporation Developer Edition (64-bit) on Windows 10 Pro 10.0 (Build 22000: ) (Hypervisor)
(but unrelated to issue)

Client Operating System

Win 11 (but unrelated)

JAVA/JVM version

17.0.4 (but unrelated)

Problem description

This change here:

  • Added explicit dependency for com.microsoft.azure.msal4j (was a transitive dependency in previous releases) 1863

introduced an explicit dependency on msal4j (which is good compared to the transitive before), but seems to have missed to flag it as optional, like the others.
ef14c5b

As a result, the driver will not deploy anymore in an OSGi environment without that dependency:
org.apache.felix.resolver.reason.ReasonException: Unable to resolve com.microsoft.sqlserver.mssql-jdbc/11.2.0: missing requirement [com.microsoft.sqlserver.mssql-jdbc/11.2.0] osgi.wiring.package; filter:="(&(osgi.wiring.package=com.microsoft.aad.msal4j)(version>=1.13.0)(!(version>=2.0.0)))"

I think this was just missed, since the optional check is still there:

Expected behavior

Dependency declared optional and deploying without successfully.

@MrEasy
Copy link
Contributor Author

MrEasy commented Aug 9, 2022

PR: #1893
Works fine again with this change.

@Jeffery-Wasty
Copy link
Contributor

Hi @MrEasy,

Thanks for bringing this to our attention. You may be right, I'll look through our requirements again, but this should be a quick yes/no.

@lilgreenbird
Copy link
Contributor

lilgreenbird commented Aug 9, 2022

hi @MrEasy
Thank you for bringing this to our attention. You are correct we will get this fixed in an upcoming 11.2.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants