use latest config in S3Path's unless passed explicitly

Project.toml

@@ -5,6 +5,7 @@ version = "0.8.7"
 [deps]
 AWS = "fbe9abb3-538b-5e4e-ba9e-bc94f4f92ebc"
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"
+Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
 EzXML = "8f5d6c58-4d21-5cfd-889c-e3ad7ee6a615"
 FilePathsBase = "48062228-2e41-5def-b9a4-89aafe57970f"
@@ -18,6 +19,7 @@ XMLDict = "228000da-037f-5747-90a9-8195ccbf91a5"
 
 [compat]
 AWS = "1.25"
+Compat = "3.29.0"
 EzXML = "0.9, 1"
 FilePathsBase = "0.9"
 HTTP = "0.8, 0.9"

src/AWSS3.jl

@@ -32,6 +32,7 @@ using EzXML
 using Dates
 using Base64
 using UUIDs
+using Compat: @something
 
 @service S3
 

src/s3path.jl

@@ -1,4 +1,6 @@
-struct S3Path{A<:AbstractAWSConfig} <: AbstractPath
+const S3PathConfig = Union{AbstractAWSConfig, Nothing}
+
+struct S3Path{A<:S3PathConfig} <: AbstractPath
     segments::Tuple{Vararg{String}}
     root::String
     drive::String
@@ -9,13 +11,13 @@ end
 
 # constructor that converts but does not require type parameter
 function S3Path(segments, root::AbstractString, drive::AbstractString, isdirectory::Bool,
-                config::AbstractAWSConfig, version::AbstractS3Version=nothing)
+                config::S3PathConfig, version::AbstractS3Version=nothing)
     S3Path{typeof(config)}(segments, root, drive, isdirectory, config, version)
 end
 
 """
     S3Path()
-    S3Path(str; config::AbstractAWSConfig=aws_config(), version=nothing)
+    S3Path(str; config::$(S3PathConfig)=nothing, version=nothing)
 
 Construct a new AWS S3 path type which should be of the form
 "s3://<bucket>/prefix/to/my/object".
@@ -32,30 +34,32 @@ NOTES:
 - On top of the standard path properties (e.g., `segments`, `root`, `drive`,
   `separator`), `S3Path`s also support `bucket` and `key` properties for your
   convenience.
+- if `config` is left at its default value of `nothing`, then the
+  latest `global_aws_config()` will be used in any operations involving the
+  path. To "freeze" the config at construction time, explicitly pass an
+  `AbstractAWSConfig` to the `config` keyword argument.
 - If `version` argument is `nothing`, will return latest version of object.
 """
 function S3Path()
-    config = global_aws_config()
-    account_id = aws_account_number(config)
-    region = config.region
+    config = nothing
 
     return S3Path(
         (),
         "/",
-        "s3://$account_id-$region",
+        "",
         true,
         config,
         nothing,
     )
 end
 # below definition needed by FilePathsBase
-S3Path{A}() where {A<:AbstractAWSConfig} = S3Path()
+S3Path{A}() where {A<:S3PathConfig} = S3Path()
 
 function S3Path(
     bucket::AbstractString,
     key::AbstractString;
     isdirectory::Bool=false,
-    config::AbstractAWSConfig=global_aws_config(),
+    config::S3PathConfig=nothing,
     version::AbstractS3Version=nothing,
 )
     return S3Path(
@@ -72,7 +76,7 @@ function S3Path(
     bucket::AbstractString,
     key::AbstractPath;
     isdirectory::Bool=false,
-    config::AbstractAWSConfig=global_aws_config(),
+    config::S3PathConfig=nothing,
     version::AbstractS3Version=nothing,
 )
     return S3Path(
@@ -86,8 +90,7 @@ function S3Path(
 end
 
 # To avoid a breaking change.
-function S3Path(str::AbstractString; config::AbstractAWSConfig=global_aws_config(),
-                version::AbstractS3Version=nothing)
+function S3Path(str::AbstractString; config::S3PathConfig=nothing, version::AbstractS3Version=nothing)
     result = tryparse(S3Path, str; config=config)
     result !== nothing || throw(ArgumentError("Invalid s3 path string: $str"))
     if version !== nothing && !isempty(version)
@@ -97,12 +100,9 @@ function S3Path(str::AbstractString; config::AbstractAWSConfig=global_aws_config
     return result
 end
 
-# if config=nothing, will not try to talk to AWS until after string is confirmed to be an s3 path
 function Base.tryparse(::Type{<:S3Path}, str::AbstractString; config::Union{Nothing,AbstractAWSConfig}=nothing)
     str = String(str)
     startswith(str, "s3://") || return nothing
-    # we do this here so that the `@p_str` macro only tries to call AWS if it actually has an S3 path
-    (config ≡ nothing) && (config = global_aws_config())
     root = ""
     path = ()
     isdirectory = true
@@ -187,7 +187,11 @@ function FilePathsBase.parents(fp::S3Path)
     end
 end
 
-FilePathsBase.exists(fp::S3Path) = s3_exists(fp.config, fp.bucket, fp.key; version=fp.version)
+# Use `fp.config` unless it is nothing; in that case, get the latest `global_aws_config`
+get_config(fp::S3Path) = @something(fp.config, global_aws_config())
+
+FilePathsBase.exists(fp::S3Path) = s3_exists(get_config(fp), fp.bucket, fp.key; version=fp.version)
+
 Base.isfile(fp::S3Path) = !fp.isdirectory && exists(fp)
 function Base.isdir(fp::S3Path)
     if isempty(fp.segments)
@@ -198,7 +202,7 @@ function Base.isdir(fp::S3Path)
         return false
     end
 
-    objects = s3_list_objects(fp.config, fp.bucket, key; max_items=1)
+    objects = s3_list_objects(get_config(fp), fp.bucket, key; max_items=1)
 
     # `objects` is a `Channel`, so we call iterate to see if there are any objects that
     # match our directory key.
@@ -209,7 +213,7 @@ end
 
 function FilePathsBase.walkpath(fp::S3Path; kwargs...)
     # Select objects with that prefix
-    objects = s3_list_objects(fp.config, fp.bucket, fp.key; delimiter="")
+    objects = s3_list_objects(get_config(fp), fp.bucket, fp.key; delimiter="")
 
     # Construct a new Channel using a recursive internal `_walkpath!` function
     return Channel(ctype=typeof(fp)) do chnl
@@ -276,7 +280,8 @@ function Base.stat(fp::S3Path)
     last_modified = DateTime(0)
 
     if exists(fp)
-        resp = s3_get_meta(fp.config, fp.bucket, fp.key; version=fp.version)
+        resp = s3_get_meta(get_config(fp), fp.bucket, fp.key; version=fp.version)
+
         # Example: "Thu, 03 Jan 2019 21:09:17 GMT"
         last_modified = DateTime(
             resp["Last-Modified"][1:end-4],
@@ -332,7 +337,7 @@ function Base.rm(fp::S3Path; recursive=false, kwargs...)
     end
 
     @debug "delete: $fp"
-    s3_delete(fp.config, fp.bucket, fp.key; version=fp.version)
+    s3_delete(get_config(fp), fp.bucket, fp.key; version=fp.version)
 end
 
 # We need to special case sync with S3Paths because of how directories
@@ -451,7 +456,7 @@ function Base.readdir(fp::S3Path; join=false, sort=true)
                 if !isempty(token)
                     params["continuation-token"] = token
                 end
-                S3.list_objects_v2(fp.bucket, params; aws_config=fp.config)
+                S3.list_objects_v2(fp.bucket, params; aws_config=get_config(fp))
             catch e
                 @delay_retry if ecode(e) in ["NoSuchBucket"] end
             end
@@ -472,7 +477,7 @@ function Base.readdir(fp::S3Path; join=false, sort=true)
 end
 
 function Base.read(fp::S3Path; byte_range=nothing)
-    return Vector{UInt8}(s3_get(fp.config, fp.bucket, fp.key; raw=true, byte_range=byte_range, version=fp.version))
+    return Vector{UInt8}(s3_get(get_config(fp), fp.bucket, fp.key; raw=true, byte_range=byte_range, version=fp.version))
 end
 
 Base.write(fp::S3Path, content::String; kwargs...) = Base.write(fp, Vector{UInt8}(content); kwargs...)
@@ -482,10 +487,10 @@ function Base.write(fp::S3Path, content::Vector{UInt8}; part_size_mb=50, multipa
     MAX_HTTP_BYTES = 2147483647
     fp.version === nothing || throw(ArgumentError("Can't write to a specific object version ($(fp.version))"))
     if !multipart || length(content) < MAX_HTTP_BYTES
-        return s3_put(fp.config, fp.bucket, fp.key, content)
+        return s3_put(get_config(fp), fp.bucket, fp.key, content)
     else
         io = IOBuffer(content)
-        return s3_multipart_upload(fp.config, fp.bucket, fp.key, io, part_size_mb; other_kwargs...)
+        return s3_multipart_upload(get_config(fp), fp.bucket, fp.key, io, part_size_mb; other_kwargs...)
     end
 end
 

test/s3path.jl

@@ -437,6 +437,40 @@ function s3path_tests(config)
         end
     end
 
+    # <https://github.com/JuliaCloud/AWSS3.jl/issues/168>
+    @testset "Default `S3Path` does not hold config" begin
+        path = S3Path("s3://$(bucket_name)/test_str.txt")
+        @test path.config === nothing
+        @test AWSS3.get_config(path) !== nothing
+    end
+
+    # Minio does not care about regions, so this test doesn't work there
+    if is_aws(config)
+        @testset "Global config is not frozen at construction time" begin
+            prev_config = global_aws_config()
+
+            # Setup: create a file holding a string `abc`
+            path = S3Path("s3://$(bucket_name)/test_str.txt")
+            write(path, "abc")
+            @test read(path, String) == "abc"  # Have access to read file
+
+            alt_region = prev_config.region == "us-east-2" ? "us-east-1" : "us-east-2"
+            try
+                global_aws_config(region=alt_region) # this is the wrong region!
+                @test_throws AWS.AWSException read(path, String)
+
+                # restore the right region
+                global_aws_config(prev_config)
+                # Now it works, without recreating `path`
+                @test read(path, String) == "abc"
+                rm(path)
+            finally
+                # In case a test threw, make sure we really do restore the right global config
+                global_aws_config(prev_config)
+            end
+        end
+    end
+
     # Broken on minio 
     if is_aws(config)
         AWSS3.s3_nuke_bucket(config, bucket_name)