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

Do not encode path separator in LargeFileHelper::getFileSizeViaCurl #36319

Merged
merged 2 commits into from
Oct 24, 2019

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Oct 23, 2019

Description

  1. Do not encode path separator when encode path for cURL as this explodes with error (3) Invalid file://hostname/, expected localhost or 127.0.0.1 or none for newer libcurl (7.59+ ?)

  2. Disable the files_external samba "unit" tests that are causing problems. We can enable them again when we sort out how to get them working. But for now CI needs to pass.

Related Issue

#36322
Fixes broken master builds

Motivation and Context

Fix CI

How Has This Been Tested?

By drone

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@VicDeo VicDeo requested a review from phil-davis October 23, 2019 10:12
@VicDeo VicDeo self-assigned this Oct 23, 2019
@VicDeo VicDeo added this to the development milestone Oct 23, 2019
@VicDeo VicDeo force-pushed the fix-getFileSizeViaCurl branch from 3f54ada to e6a5c9f Compare October 23, 2019 10:22
@phil-davis phil-davis mentioned this pull request Oct 23, 2019
@codecov
Copy link

codecov bot commented Oct 23, 2019

Codecov Report

Merging #36319 into master will decrease coverage by 0.3%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #36319      +/-   ##
============================================
- Coverage     65.16%   64.86%   -0.31%     
  Complexity    19775    19775              
============================================
  Files          1271     1271              
  Lines         74703    74705       +2     
  Branches       1309     1309              
============================================
- Hits          48679    48455     -224     
- Misses        25638    25864     +226     
  Partials        386      386
Flag Coverage Δ Complexity Δ
#javascript 54% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.05% <100%> (-0.34%) 19775 <0> (ø)
Impacted Files Coverage Δ Complexity Δ
lib/private/LargeFileHelper.php 80.7% <100%> (+0.7%) 23 <0> (ø) ⬇️
apps/files_external/lib/Lib/Storage/SMB.php 0% <0%> (-52.44%) 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 822f3e6...40955f0. Read the comment docs.

@phil-davis phil-davis force-pushed the fix-getFileSizeViaCurl branch from ac40bc8 to 40955f0 Compare October 24, 2019 08:41
@phil-davis
Copy link
Contributor

I disabled the failing files_external samba tests. Those can be enabled again when the issue with /var/cache/samba is sorted out. But for now IMO it will be better to have CI working, rather than blocked.

@VicDeo VicDeo merged commit 1424918 into master Oct 24, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix-getFileSizeViaCurl branch October 24, 2019 10:05
@phil-davis phil-davis mentioned this pull request Nov 5, 2019
11 tasks
@davitol davitol mentioned this pull request Nov 26, 2019
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants