Skip to content

Conversation

@Jeffery-Wasty
Copy link
Contributor

The logic for finding the mssql-jdbc.properties file, used in Configurable Retry Logic was correct for test environments, and when building the driver manually, but not correct when using the driver as a jar or dependency. This fixes that issue by only removing the target/classes suffix if needed.

@Jeffery-Wasty Jeffery-Wasty modified the milestone: 12.10.0 Jan 9, 2025
@Jeffery-Wasty Jeffery-Wasty linked an issue Jan 9, 2025 that may be closed by this pull request
@Jeffery-Wasty Jeffery-Wasty self-assigned this Jan 9, 2025
@Jeffery-Wasty Jeffery-Wasty added the Under Review Used for pull requests under review label Jan 10, 2025
@codecov
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 51.37%. Comparing base (08cd6fd) to head (acef949).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...crosoft/sqlserver/jdbc/ConfigurableRetryLogic.java 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2579      +/-   ##
============================================
+ Coverage     51.22%   51.37%   +0.15%     
- Complexity     3953     3978      +25     
============================================
  Files           147      147              
  Lines         33657    33665       +8     
  Branches       5624     5627       +3     
============================================
+ Hits          17241    17296      +55     
+ Misses        13999    13936      -63     
- Partials       2417     2433      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

machavan
machavan previously approved these changes Feb 6, 2025
@Jeffery-Wasty Jeffery-Wasty requested a review from Copilot February 6, 2025 16:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again, by re-requesting a review.

@Jeffery-Wasty Jeffery-Wasty changed the title Fix for broken mssql-jdbc.properties location logic Fix for mssql-jdbc.properties location logic Feb 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java:287

  • The variable 'locationSuffix' is defined but not used consistently. Consider using 'locationSuffix.length()' instead of hardcoding the length value (16) on line 291.
String locationSuffix = "target/classes/";

@machavan
Copy link
Contributor

ADO test runs were good for this change.

@machavan machavan merged commit 83072af into main Feb 11, 2025
18 of 19 checks passed
@Jeffery-Wasty Jeffery-Wasty removed the Under Review Used for pull requests under review label Feb 11, 2025
@Jeffery-Wasty Jeffery-Wasty deleted the CRLFileLocFix branch February 11, 2025 20:02
@lilgreenbird lilgreenbird changed the title Fix for mssql-jdbc.properties location logic Fixed issue for finding mssql-jdbc.properties location in test environments Mar 4, 2025
@Dmole Dmole mentioned this pull request May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed/Merged PRs

Development

Successfully merging this pull request may close these issues.

Broken logic in ConfigurableRetryLogic

8 participants