From 631dbb889c094a304178ef664e035e466a54164a Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 13:50:09 +0100 Subject: [PATCH 1/8] Always close the `response_stream` - We need to explicitly close the `response_stream` (`HTTP.request` no longer closes it for us) in HTTP.jl v0.9.15+ (see HTTP.jl #752) - Since `close` is safe to call multiple times, this should be compatible with old HTTP.jl versions. - Should fix issue #466 --- src/utilities/request.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 12debd498b..1e8e176bc7 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -213,6 +213,8 @@ function _http_request(http_backend::HTTPBackend, request::Request) isa(e, Base.IOError) || (isa(e, HTTP.StatusError) && _http_status(e) >= 500) end + finally + close(request.response_stream) end end From 65da8571eec365a6114e98157c09d06b3871a301 Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 14:43:01 +0100 Subject: [PATCH 2/8] Only try to close an actual stream Co-authored-by: Fredrik Ekre --- src/utilities/request.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/request.jl b/src/utilities/request.jl index 1e8e176bc7..a2e7dee1ba 100644 --- a/src/utilities/request.jl +++ b/src/utilities/request.jl @@ -214,7 +214,7 @@ function _http_request(http_backend::HTTPBackend, request::Request) (isa(e, HTTP.StatusError) && _http_status(e) >= 500) end finally - close(request.response_stream) + request.response_stream isa IO && close(request.response_stream) end end From 4dcecc6686537b563b953010fbee2b2e16efc07a Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 15:37:52 +0100 Subject: [PATCH 3/8] Add test --- test/issues.jl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/issues.jl b/test/issues.jl index 469e66be4c..bf8f1535ee 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -81,3 +81,24 @@ end S3.delete_bucket(bucket_name) end end + +@testset "issue 466" begin + bucket_name = "aws-jl-test-issues---" * _now_formatted() + file_name = "hang.txt" + body = "Hello World!" + S3.create_bucket(bucket_name) + S3.put_object(bucket_name, file_name, Dict("body" => body)) + + try + S3.create_bucket(bucket_name) + S3.put_object(bucket_name, file_name) + + stream = S3.get_object(bucket_name, file_name, Dict("return_stream" => true)) + println("test #466") # So we know if this is the reason for tests hanging. + @test eof(stream) # This will hang if #466 not fixed and using HTTP.jl v0.9.15+ + println("#466 fixed") + finally + S3.delete_object(bucket_name, file_name) + S3.delete_bucket(bucket_name) + end +end From 02687e683977a742f99edb1669fe1cce5a180c5a Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 16:53:27 +0100 Subject: [PATCH 4/8] Don't keep creating and deleting bucekts in `issues` tests --- test/issues.jl | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/test/issues.jl b/test/issues.jl index bf8f1535ee..69be9889ce 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -1,7 +1,7 @@ @service S3 -bucket_name = "aws-jl-test-issues---" * _now_formatted() -S3.create_bucket(bucket_name) +const BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() +S3.create_bucket(BUCKET_NAME) @testset "issue 223" begin # https://github.com/JuliaCloud/AWS.jl/issues/223 @@ -9,13 +9,12 @@ S3.create_bucket(bucket_name) file_name = "contains spaces" try - S3.put_object(bucket_name, file_name, Dict("body" => body)) - resp = S3.get_object(bucket_name, file_name) + S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) + resp = S3.get_object(BUCKET_NAME, file_name) @test String(resp) == body finally - S3.delete_object(bucket_name, file_name) - S3.delete_bucket(bucket_name) + S3.delete_object(BUCKET_NAME, file_name) end end @@ -59,16 +58,15 @@ end file_name = "streaming.bin" try - S3.create_bucket(bucket_name) - S3.put_object(bucket_name, file_name, Dict("body" => body)) - resp = S3.get_object(bucket_name, file_name) + S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) + resp = S3.get_object(BUCKET_NAME, file_name) @test String(resp) == body # ERROR: MethodError: no method matching iterate(::Base.BufferStream) # => BUG: header `response_stream` is pushed into the query... io = Base.BufferStream() S3.get_object( - bucket_name, file_name, Dict("response_stream" => io, "return_stream" => true) + BUCKET_NAME, file_name, Dict("response_stream" => io, "return_stream" => true) ) if bytesavailable(io) > 0 @test String(readavailable(io)) == body @@ -77,28 +75,22 @@ end end finally - S3.delete_object(bucket_name, file_name) - S3.delete_bucket(bucket_name) + S3.delete_object(BUCKET_NAME, file_name) end end @testset "issue 466" begin - bucket_name = "aws-jl-test-issues---" * _now_formatted() file_name = "hang.txt" - body = "Hello World!" - S3.create_bucket(bucket_name) - S3.put_object(bucket_name, file_name, Dict("body" => body)) try - S3.create_bucket(bucket_name) - S3.put_object(bucket_name, file_name) - - stream = S3.get_object(bucket_name, file_name, Dict("return_stream" => true)) + S3.put_object(BUCKET_NAME, file_name) + stream = S3.get_object(BUCKET_NAME, file_name, Dict("return_stream" => true)) println("test #466") # So we know if this is the reason for tests hanging. @test eof(stream) # This will hang if #466 not fixed and using HTTP.jl v0.9.15+ println("#466 fixed") finally - S3.delete_object(bucket_name, file_name) - S3.delete_bucket(bucket_name) + S3.delete_object(BUCKET_NAME, file_name) end end + +S3.delete_bucket(BUCKET_NAME) From 27dfaee697cd1d1f223cbe9f3c98657fdfc9d8c9 Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 17:52:54 +0100 Subject: [PATCH 5/8] Wrap tests in try/finally so `delete_bucket` always called --- test/issues.jl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/issues.jl b/test/issues.jl index 69be9889ce..aeb8cebf34 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -1,7 +1,9 @@ @service S3 const BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() -S3.create_bucket(BUCKET_NAME) + +try + S3.create_bucket(BUCKET_NAME) @testset "issue 223" begin # https://github.com/JuliaCloud/AWS.jl/issues/223 @@ -93,4 +95,6 @@ end end end -S3.delete_bucket(BUCKET_NAME) +finally + S3.delete_bucket(BUCKET_NAME) +end From f0d6e11b3c57b7dd8aea117e9a5b7dd0a4eecf03 Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 17:56:49 +0100 Subject: [PATCH 6/8] try to make formatter happy --- test/issues.jl | 146 ++++++++++++++++++++++++------------------------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/test/issues.jl b/test/issues.jl index aeb8cebf34..185caf7df8 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -5,95 +5,95 @@ const BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() try S3.create_bucket(BUCKET_NAME) -@testset "issue 223" begin - # https://github.com/JuliaCloud/AWS.jl/issues/223 - body = "Hello World!" - file_name = "contains spaces" - - try - S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) - resp = S3.get_object(BUCKET_NAME, file_name) - - @test String(resp) == body - finally - S3.delete_object(BUCKET_NAME, file_name) - end -end + @testset "issue 223" begin + # https://github.com/JuliaCloud/AWS.jl/issues/223 + body = "Hello World!" + file_name = "contains spaces" -@testset "issue 227" begin - @testset "s3 public bucket" begin - # https://github.com/JuliaCloud/AWS.jl/issues/227 - config = AWSConfig(; creds=nothing) - resp = S3.get_object("www.invenia.ca", "index.html"; aws_config=config) + try + S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) + resp = S3.get_object(BUCKET_NAME, file_name) - @test !isempty(resp) + @test String(resp) == body + finally + S3.delete_object(BUCKET_NAME, file_name) + end end - @testset "s3 private bucket" begin - bucket_name = "aws-jl-test-issues---" * _now_formatted() - file_name = "hello_world" + @testset "issue 227" begin + @testset "s3 public bucket" begin + # https://github.com/JuliaCloud/AWS.jl/issues/227 + config = AWSConfig(; creds=nothing) + resp = S3.get_object("www.invenia.ca", "index.html"; aws_config=config) - try - S3.create_bucket(bucket_name) - S3.put_object(bucket_name, file_name) + @test !isempty(resp) + end + + @testset "s3 private bucket" begin + bucket_name = "aws-jl-test-issues---" * _now_formatted() + file_name = "hello_world" + + try + S3.create_bucket(bucket_name) + S3.put_object(bucket_name, file_name) + + @test_throws AWSException S3.get_object( + bucket_name, file_name; aws_config=AWSConfig(; creds=nothing) + ) + finally + S3.delete_object(bucket_name, file_name) + S3.delete_bucket(bucket_name) + end + end - @test_throws AWSException S3.get_object( - bucket_name, file_name; aws_config=AWSConfig(; creds=nothing) + @testset "lambda" begin + @service Lambda + + @test_throws NoCredentials Lambda.list_functions(; + aws_config=AWSConfig(; creds=nothing) ) - finally - S3.delete_object(bucket_name, file_name) - S3.delete_bucket(bucket_name) end end - @testset "lambda" begin - @service Lambda + @testset "issue 324" begin + body = "Hello World!" + file_name = "streaming.bin" - @test_throws NoCredentials Lambda.list_functions(; - aws_config=AWSConfig(; creds=nothing) - ) - end -end + try + S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) + resp = S3.get_object(BUCKET_NAME, file_name) + @test String(resp) == body + + # ERROR: MethodError: no method matching iterate(::Base.BufferStream) + # => BUG: header `response_stream` is pushed into the query... + io = Base.BufferStream() + S3.get_object( + BUCKET_NAME, file_name, Dict("response_stream" => io, "return_stream" => true) + ) + if bytesavailable(io) > 0 + @test String(readavailable(io)) == body + else + @test "no body data was available" == body + end -@testset "issue 324" begin - body = "Hello World!" - file_name = "streaming.bin" - - try - S3.put_object(BUCKET_NAME, file_name, Dict("body" => body)) - resp = S3.get_object(BUCKET_NAME, file_name) - @test String(resp) == body - - # ERROR: MethodError: no method matching iterate(::Base.BufferStream) - # => BUG: header `response_stream` is pushed into the query... - io = Base.BufferStream() - S3.get_object( - BUCKET_NAME, file_name, Dict("response_stream" => io, "return_stream" => true) - ) - if bytesavailable(io) > 0 - @test String(readavailable(io)) == body - else - @test "no body data was available" == body + finally + S3.delete_object(BUCKET_NAME, file_name) end - - finally - S3.delete_object(BUCKET_NAME, file_name) end -end -@testset "issue 466" begin - file_name = "hang.txt" - - try - S3.put_object(BUCKET_NAME, file_name) - stream = S3.get_object(BUCKET_NAME, file_name, Dict("return_stream" => true)) - println("test #466") # So we know if this is the reason for tests hanging. - @test eof(stream) # This will hang if #466 not fixed and using HTTP.jl v0.9.15+ - println("#466 fixed") - finally - S3.delete_object(BUCKET_NAME, file_name) + @testset "issue 466" begin + file_name = "hang.txt" + + try + S3.put_object(BUCKET_NAME, file_name) + stream = S3.get_object(BUCKET_NAME, file_name, Dict("return_stream" => true)) + println("test #466") # So we know if this is the reason for tests hanging. + @test eof(stream) # This will hang if #466 not fixed and using HTTP.jl v0.9.15+ + println("#466 fixed") + finally + S3.delete_object(BUCKET_NAME, file_name) + end end -end finally S3.delete_bucket(BUCKET_NAME) From 817ba0cc27b7a9716affdb875b2c4e76172bfe93 Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 18:01:09 +0100 Subject: [PATCH 7/8] try again to make formatter happy --- test/issues.jl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/issues.jl b/test/issues.jl index 185caf7df8..df28560c9e 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -68,7 +68,9 @@ try # => BUG: header `response_stream` is pushed into the query... io = Base.BufferStream() S3.get_object( - BUCKET_NAME, file_name, Dict("response_stream" => io, "return_stream" => true) + BUCKET_NAME, + file_name, + Dict("response_stream" => io, "return_stream" => true), ) if bytesavailable(io) > 0 @test String(readavailable(io)) == body From 8dde0b0eb34e836033aec211bc1b4ec8c613b45d Mon Sep 17 00:00:00 2001 From: Nick Robinson Date: Tue, 28 Sep 2021 19:09:39 +0100 Subject: [PATCH 8/8] Update test/issues.jl --- test/issues.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/issues.jl b/test/issues.jl index df28560c9e..dfdeef8dc9 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -1,6 +1,6 @@ @service S3 -const BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() +BUCKET_NAME = "aws-jl-test-issues---" * _now_formatted() try S3.create_bucket(BUCKET_NAME)