Support specifying features with @service
#457
Conversation
|
bors try |
tryBuild failed: |
|
There's a expression corner case on Julia 1.4 and below which I didn't capture in my tests. |
|
bors try |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Last change just fixes formatting so I won't bother re-running bors |
|
I can apply the fixups prior to merging once approved |
|
bors try |
|
In experimenting with this in #458 it's probably sensible to have the low-level API also make use of features |
As the low-level API is from |
|
Rebasing fixups |
omus
force-pushed
the
cv/service-features
branch
from
dd90a1c to
cccc439
Compare
|
bors try |
src/api_generation/high_level.jl
Outdated
| $formatted_function_name($(join(req_keys, ", ")); aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", $params_headers_str; aws_config=aws_config, features=SERVICE_FEATURES) | ||
| $formatted_function_name($(join(req_keys, ", ")), params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", Dict{String, Any}(mergewith(_merge, $params_headers_str, params)); aws_config=aws_config, features=SERVICE_FEATURES) |
There was a problem hiding this comment.
An alternative approach would be to define the following right after the generated using statements:
const $service_name = set_features(AWS.AWSServices.$service_name; SERVICE_FEATURES...)This would reduce the changes to the generated code at the cost of having to unpack the ServiceWrapper. Probably the current approach in the PR is the right one.
|
bors try |
|
I'll mention that #460 wouldn't play well with the high-level service features as this PR currently implements them. I think the way these two things could compose is that AWS.jl would have the high-level services as submodules. If you want to use the high-level services with non-default features you would need to use the |
|
Taking this out of our discussion from Slack before it gets lost into the void. This seems like a good step forward with allowing for breaking changes to occur in this package without releasing a new major version. When we eventually get to N amount of breaking changes, or just choose to make a new release it should be easy enough to incorporate. Especially since work in this package is kind of sporadic. The code itself LGTM! The trade off of more generated code vs. unpacking makes sense, no one will be looking at the generated code regardless. |
omus
force-pushed
the
cv/service-features
branch
from
edcd935 to
10b07ea
Compare
|
Rebased to resolve conflict and I squashed some commits |
|
bors r+ |
384: Use `AWS.Response` to handle streaming/raw/parsing r=omus a=omus The idea is to use a struct in AWS.jl that can be used to handle the automatic parsing that is currently used to turn XML/JSON into a dict while also giving the option of accessing the raw output as a string or stream without all the keywords currently needed to be specified. Depends on: - #457 Related: - #346 - #348 - #438 Closes: - #224 - #433 - #468 (when using `use_response_type` the passed in I/O is not closed) - #471 - #433 Update: The tests in this PR run using the deprecated behaviour. Mainly I did that here to prove this change is non-breaking. For the updated tests see #458 Co-authored-by: Curtis Vogt <[email protected]>
While working on #384 I encountered a situation where the change I was making was breaking so in order to have to make a major release I wanted to make the change opt-in. Unfortunately doing that would result in every single AWS.jl API call being modified to opt-in to the new behaviour making updating quite tedious.
The
@servicefeature support added in this PR allows end users to opt-in to new behaviour when using the@servicemacro (e.g.@service S3 my_new_feature=true). This interface is much more convenient and probably is a pattern we want to use for incorporating future behaviour. Currently this PR defines no features. Additionally for low-level API calls I've introduced theset_featuresfunction (e.g.set_features(::Service; my_new_feature=true)) which allows for setting the feature set to be used for a service instance.The only issue I can think of with this is if an end user makes use of multiple
@servicecalls to the same service in the same module then the last call will define the behaviour. As doing this will result in a module being redefined I don't think this is a situation anyone will notice.