Added testsets and an upload function that can do retries #42
Conversation
src/FTPObject.jl
Outdated
| """ | ||
| function upload( | ||
| ftp::FTP, | ||
| local_file_paths::Vector{<:AbstractString}, |
There was a problem hiding this comment.
This syntax is only available on Julia 0.6 and later. You should be able to do this if you write:
@compat function upload(
ftp::FTP,
local_file_paths::Vector{<:AbstractString},
...
Codecov Report
@@ Coverage Diff @@
## master #42 +/- ##
==========================================
- Coverage 90.71% 90.69% -0.02%
==========================================
Files 3 3
Lines 237 258 +21
==========================================
+ Hits 215 234 +19
- Misses 22 24 +2
Continue to review full report at Codecov.
|
src/FTPC.jl
Outdated
| @@ -125,9 +127,9 @@ function write_file_cb(buff::Ptr{UInt8}, sz::Csize_t, n::Csize_t, p_wd::Ptr{Void | |||
| nbytes::Csize_t | |||
| end | |||
|
|
|||
| c_write_file_cb = cfunction(write_file_cb, Csize_t, (Ptr{UInt8}, Csize_t, Csize_t, Ptr{Void})) | |||
| c_write_file_cb = cfunction(write_file_cb, Csize_t, Tuple{Ptr{UInt8}, Csize_t, Csize_t, Ptr{Nothing}}) | |||
There was a problem hiding this comment.
The deprecation warning is a bit misleading. For C calls you should use Cvoid. That's usually the case when you do Ptr{Void}.
| @testset "conn error" begin | ||
| # check connection error | ||
| @test_throws FTPClientError FTP(hostname="not a host", username=username(server), password=password(server), ssl=false) | ||
| end |
There was a problem hiding this comment.
Did you just add @testsets here? Or are there other changes I should look for?
There was a problem hiding this comment.
As far as I can tell, adding @testsets was the only change here
| @@ -1,6 +1,8 @@ | |||
| using Compat | |||
There was a problem hiding this comment.
You should set the minimum required version of Compat to v0.41 (in the REQUIRE file). This is the first version of Compat that includes Nothing and Cvoid.
src/FTPC.jl
Outdated
| @@ -113,7 +115,7 @@ end | |||
| # Callbacks | |||
| ############################## | |||
|
|
|||
| function write_file_cb(buff::Ptr{UInt8}, sz::Csize_t, n::Csize_t, p_wd::Ptr{Void}) | |||
| function write_file_cb(buff::Ptr{UInt8}, sz::Csize_t, n::Csize_t, p_wd::Ptr{Nothing}) | |||
There was a problem hiding this comment.
Since you're just C calls here you should be using Ptr{Cvoid} instead of Ptr{Nothing}. Details in the Julia NEWS
Voidhas been renamed back toNothingwith an aliasCvoidfor use when calling C with a return type ofCvoidor a return or argument type ofPtr{Cvoid}(#25162).
src/FTPObject.jl
Outdated
| retry_wait_seconds::Integer = 5 | ||
| ) | ||
|
|
||
| successful_delivery = Vector{Bool}() |
There was a problem hiding this comment.
Could be written simply as successful_delivery = Bool[]
| attempts += 1 | ||
| end | ||
|
|
||
| push!(successful_delivery, file_delivery_success) |
There was a problem hiding this comment.
Maybe just return file_delivery_success. Then you can have successful_delivery = open(...) above
There was a problem hiding this comment.
I'm not sure what your suggestion is here. The function is attempting to upload multiple files so returns successful_delivery, a vector of booleans that indicate if the file was successfully delivered.
There was a problem hiding this comment.
I stand corrected. I double checked this but I missed the for loop. Sorry for the noise
REQUIRE
Outdated
| @@ -1,3 +1,3 @@ | |||
| julia 0.6 | |||
| LibCURL 0.2.2 | |||
| Compat 0.8.3 | |||
| Compat 0.41- | |||
There was a problem hiding this comment.
Just Compat 0.41 is good here
There was a problem hiding this comment.
I should reword the above to be:
This should be changed to be Compat 0.41
|
Feel free to make a merge commit whenever. Avoid using squash and merge here as there are two distinct authors. |
-new upload function
-tests for new upload function
-test sets for ftp_object