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

Define AWSConfig as a struct? #53

Open
morris25 opened this issue Jan 11, 2019 · 5 comments
Open

Define AWSConfig as a struct? #53

morris25 opened this issue Jan 11, 2019 · 5 comments
Assignees
Labels
Milestone

Comments

@morris25
Copy link
Contributor

Considering the existence of aws_config(), it seems to me that it would make more sense to define AWSConfig as a struct rather than a SymbolDict.

I'm curious as to why this design decision was made and whether people think this change may or may not be a good idea.

ararslan added a commit that referenced this issue Feb 1, 2019
Rather than defining `AWSConfig` as a `SymbolDict` with no restrictions
on the contents, we can make it its own type, which gives more control
over how things are stored and accessed.

Fixes #53
ararslan added a commit that referenced this issue Feb 1, 2019
Rather than defining `AWSConfig` as a `SymbolDict` with no restrictions
on the contents, we can make it its own type, which gives more control
over how things are stored and accessed.

Fixes #53
@samoconnor
Copy link
Contributor

Hi @morris25, it has always been my intention to eventually replace the aws session Dict with a struct. The code was originally ported from Tcl, where there are no structs. Keeping the free-form Dict of state was a handy way to add service-specific state functionality as needed without having to re-release AWSCore.jl every time something changed.

Note that while the aws Dict starts off containing just configuration, it goes on to store state with per-session lifetime (e.g. AWSS3.jl maintains a :bucket_region cache to map bucket names to regions) and per-request lifetime (e.g. :return_stream, :response_stream, :ordered_json_dict in AWSCore.jl and special handling of :resource in AWSSQS.jl).

Over time, some stuff has been pushed into the AWSCredentials struct.
It probably makes sense to have a AWSRequest struct for per-request lifetime state,
(and an AWSSession struct).
However, we'll need to retain a mechanism for service-specific stuff.

Just now my laptop is saying 2% batt.
To be continued...

@ararslan
Copy link
Member

ararslan commented Feb 7, 2019

I agree it makes sense to have a type for AWSRequest, etc. Service-specific options could be passed as keyword arguments where necessary, e.g. create_bucket(...; bucket_region="us-east-1"). Then they can be handily accessed like NamedTuples.

@iamed2
Copy link
Member

iamed2 commented Feb 12, 2019

AWSS3 is different enough that I think service-specific stuff could go in its own type there.

We could also have an AWSService type and replace all of the specific functions we have in Services.jl. An AWSService could contain the name of the service, the type of request to make (e.g., service_query, service_rest_json), default arguments to pass to that request function (e.g., target, version), and service-level extra data in a dict maybe. Then Services.jl is just a bunch of things like this:

const cloudformation = AWSService(service_query, "cloudformation", "2010-05-15")

and then a generic call method defined on instances of AWSService. Or instead of a generic call method maybe AWSService is abstract and you have subtypes like JSONService and QueryService that have their own call methods (meaning they can specifically look for args like verb, for example).

@samoconnor
Copy link
Contributor

I would very much like to retain the current scheme where there is a function for each service in Services.jl (rather than have a global request function and force users to construct a typed service object so that request can dispatch to the right place).
As things stand, the 1st argument (aws) is optional, e.g.:

ec2(operation, args=[]) =
    ec2(default_aws_config(), operation, args)

This supports code like this where use of the default config is completely hidden (this pattern is very common in scripts that run on EC2 or lambda, where the config and credentials are inherited from the hosting container.

using AWSCore.Services.ec2
ec2("CreateVpc", CidrBlock = cidr)
ec2("AttachInternetGateway", VpcId = vpc_id, InternetGatewayId = gw)

Another common pattern is to have two different aws config dicts configured for different accounts.
It is convenient to use each to call a few different AWS services. e.g.:

us = aws_config()
them = JSON.parse(3rd_party_aws_config)

for x in 3rd_party_stuff_to_import
    bar = dynamodb(them, ... look up info in x ...)
    foo = s3(them, "GET", bar)
    s3(us, "PUT", x, foo)
    sqs(us, "SendMessage", ...)
end

This makes things simpler than having to create a seperate service objects for dynamo_them, s3_them, s3_us and sqs_us.

@iamed2
Copy link
Member

iamed2 commented Feb 14, 2019

Right, what I was suggesting would not change the user interface at all. I'll give a detailed example.

module AWSCore

# ...

abstract type AWSService end

mutable struct QueryService
    service::String
    version::String
    extra_data_maybe::Dict
end

QueryService(service, version) = QueryService(service, version, Dict())

function (qs::QueryService)(aws::AWSConfig, operation, args=[])
    service_query(
        aws;
        service=qs.service,
        version=qs.version,
        operation=operation,
        args=args,
    )
end

(qs::QueryService)(operation, args=[]) = qs(default_aws_config(), operation, args)
(qs::QueryService)(a...; b...) = qs(a..., b)

module Services

using ..AWSCore

const cloudformation = AWSCore.QueryService("cloudformation", "2010-05-15")

# ...

end

end

and user code would look the same:

using AWSCore.Services: cloudformation
cloudformation("DescribeStackOutput", StackName="MyStack")
cloudformation(aws_config(profile="other"), "SomeOperation", some_args)

@mattBrzezinski mattBrzezinski added this to the AWSCore v2.0 milestone Jan 30, 2020
@mattBrzezinski mattBrzezinski self-assigned this Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants