Skip to content

Fix issue#9807 review 2nd (Replaces PR #11867) DRAFT #12489

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

Draft
wants to merge 226 commits into
base: main
Choose a base branch
from

Conversation

paul1956
Copy link
Contributor

@paul1956 paul1956 commented Nov 15, 2024

Fixes #9807

Proposed changes

  • Add Remove dependance on WebClient for VB FileDownload by using HttpClient
  • Upload still uses WebClient that also needs to be done as a separate PR

Customer Impact

  • Adds Async file download support to VB, and if WebClient goes away this will allow existing Visual Basic programs using synchronous File Download to still work.
  • It also enables Async FileDownload is API is approved

Regression?

  • Yes, when WebClient if and when goes away, not today

Risk

  • The existing code uses ICredentials in some of the paths, it is unclear how compatible that is between WebClient and HttpClient

  • Close to 100% Code Coverage Before and After change, original tests had no tests and there Error Message handling was odd

  • There are Exception Messages that need to move from WebClient to this PR and be translated.

Microsoft Reviewers: Open in CodeFlow

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 94.72256% with 214 lines in your changes missing coverage. Please review.

Project coverage is 77.11603%. Comparing base (fbd28a7) to head (2f6a941).

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #12489         +/-   ##
===================================================
+ Coverage   76.90985%   77.11603%   +0.20617%     
===================================================
  Files           3260        3270         +10     
  Lines         643088      646894       +3806     
  Branches       47601       47907        +306     
===================================================
+ Hits          494598      498859       +4261     
+ Misses        144830      144362        -468     
- Partials        3660        3673         +13     
Flag Coverage Δ
Debug 77.11603% <94.72256%> (+0.20617%) ⬆️
integration 18.96574% <0.00000%> (-0.02879%) ⬇️
production 51.86075% <83.08458%> (+0.27853%) ⬆️
test 97.42366% <97.60074%> (+0.00213%) ⬆️
unit 49.25327% <83.08458%> (+0.29267%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@paul1956
Copy link
Contributor Author

@Tanya-Solyanik this PR should be DRAFT, please.

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

Successfully merging this pull request may close these issues.

Refactor VB's Application Framework Network Class
1 participant