Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix for mssql-jdbc.properties location logic #2579

Merged
merged 17 commits into from
Feb 11, 2025
Merged

Fix for mssql-jdbc.properties location logic #2579

merged 17 commits into from
Feb 11, 2025

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
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

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

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

Broken logic in ConfigurableRetryLogic
7 participants