New SMB storage backend #10673
Merged
New SMB storage backend #10673
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awesome work! I'll get testing this when I've got a proper setup again. How many issues will this close? 😆 |
|
@icewind1991 please rebase - will install libsmbclient for testing |
|
General information: linux packages are available here: http://software.opensuse.org/download.html?project=isv%3AownCloud%3Acommunity%3Atesting&package=php5-libsmbclient |
icewind1991
force-pushed
the
smb-new
branch
2 times, most recently
from
4cbd129 to
0a968c0
Compare
|
@DeepDiver1975 @PVince81 @Xenopathic please review |
|
Question: would that work on Windows as well ? I guess it would only if libsmbclient exists for Windows ? CC @MTRichards |
No this will not work on windows - windows has native support for accessing smb shares. But there are issue with connecting to the same share using mulitple credentials - this is not allowed on windows. |
|
@DeepDiver1975 @PVince81 please review |
|
Tested against my NAS 👍 |
|
Will run unit tests now ... |
|
|
Are the clocks on both machines the same? |
let me check ... also running unit tests against Windows 7 the next minutes ... |
LOL - the clock was horrible off 🙈 |
|
Against Windows 7: |
|
Error 13 seems to be permissions denied (will add handling for it). Can you try running the tests from https://github.com/icewind1991/SMB |
|
Should be revived for 8.1 |
indeed |
|
Please rebase, if it makes sense at this point |
|
Testing both the libsmbclient and the smblcient implementations of the smb library |
How to do that? |
|
enabling disabling the libsmbclient extension |
|
I guess we should follow the mechanism in 3rdparty and remove the tests folder from icewind/smb and icewind/streams @icewind1991 or maybe even you don't package test within the composer package? |
|
come on @owncloud-bot retest this please |
DeepDiver1975
force-pushed
the
smb-new
branch
from
a063a37 to
6477e39
Compare
|
rebased and tested against samba server running inside docker 👍 issues with windows are to be seen as local network/setup issues |
|
Refer to this link for build results (access rights to CI server needed): |
|
The inspection completed: 17 new issues, 12 updated code elements |
|
@PVince81 @MorrisJobke can this be merged? |
|
Yes. Tested 👍 |
|
Great job! @icewind1991 |
|
Please add as feature to https://github.com/owncloud/core/wiki/ownCloud-8.1-Features as well – THX. |
done - thx for the reminder |
|
My Questions: smbclient is caching the file to download in /tmp, which may be a problem for larger files. Is this still the case for libsmbclient ? Is the cache Folder configurable somewhere ? Currently, i am downloading a 2,15 GB File, /tmp is 1024 MB only and 100% filled right now. Will the download work or will it break ? (it is at 300 MB right now so i will know soon ..) Thanks for the Info ! Edit: On OC 8.0.3RC1, the download breaks if /tmp is full. There should be a warning or the download shall not even start for a better user experience ... Thanks ! |
ghost
mentioned this pull request
|
Ref for the documentation of this new "libsmbclient" / "libsmbclient-php" dependency: owncloud-archive/documentation#1447 It could be really useful to document something like this before the release of the version including it. |
ghost
mentioned this pull request
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New SMB storage backend based on https://github.com/icewind1991/SMB which supports either wrapping smbclient or use libsmbclient directly trough libsmbclient-php.
All tests pass locally (both with smbclient and libsmbclient)
Both methods also provide a nice speedup compared to smb4php, on my local machine I get the following times for running the smb unit tests
cc @DeepDiver1975 @PVince81 @Xenopathic
Fixes #6092, fixes #9467, fixes #9480