Skip to content

Conversation

mattBrzezinski
Copy link
Member

@mattBrzezinski mattBrzezinski commented Jun 9, 2020

TODO

I wanted to get this merge request up for review ASAP, there are some things which I know need to be addressed and can be omitted from being review:

  • Compat entries for MbedTLS, Sockets, XMLDict
  • Additional testing for src/AWS.jl functionality; _sign2(), _sign4(), etc.
  • Properly typing AWSException.info

Overview

Note this merge request is targeting the v1 branch, and does not need a version bump or anything of the sorts. This merge request introduces the re-writing for REST XML requests as part of the re-write. It also includes a re-write AWSConfig as a struct proposed by @ararslan here.

I think the best way to review this merge request is to look at the commits themselves and not the files changed. The majority of the changes are to auto-generated files such as src/AWSServices.jl and src/services/*.jl.

Next Steps

The re-write of AWS.jl is almost complete, the last few pieces of the puzzle are to:

  1. Implement the other request functionality: JSON, Query, and Rest JSON
  2. Remove all usages of AWSCore.jl and use the appropriately re-written functions
  3. Setup the GitHub action to run the auto-generation of code
  4. Setup bors, and CI, house keeping things

@ararslan
Copy link
Member

ararslan commented Jun 9, 2020

It also includes AWSConfig as proposed by @ararslan here.

I haven't been following the development of this package, but note that if AWSConfig is used in the same way it currently is in AWSCore et al., this implementation won't work; there seems to be quite a bit of existing code that assumes the AWSConfig object is a mutable AbstractDict and can store arbitrary data. Just something to keep in mind in case you weren't already cognizant of that.

@mattBrzezinski
Copy link
Member Author

It also includes AWSConfig as proposed by @ararslan here.

I haven't been following the development of this package, but note that if AWSConfig is used in the same way it currently is in AWSCore et al., this implementation won't work; there seems to be quite a bit of existing code that assumes the AWSConfig object is a mutable AbstractDict and can store arbitrary data. Just something to keep in mind in case you weren't already cognizant of that.

It's not a direct port from what you had previously proposed. I just made AWSConfig it's only mutable struct and changed places which had used the AbstractDict interface to access it's fields. Updating to clarify that!

@mattBrzezinski mattBrzezinski force-pushed the MB/rest-xml-service branch 2 times, most recently from 482956c to ed8d4f8 Compare June 17, 2020 05:30
- Mock AWS._http_request()
- Minor string performance improvements
- Formatting styles to align with BlueStyle
- AWS sign functions return the request they are modifying
- Service URL generation takes in region::String rather than
  config::AWSConfig
- Added return_headers as kwarg for AWS Requests
- Added testing for src/AWS.jl functionality
@mattBrzezinski mattBrzezinski merged commit 83a68f7 into v1 Jun 18, 2020
@mattBrzezinski mattBrzezinski deleted the MB/rest-xml-service branch June 19, 2020 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants