twisted / txaws

Twisted-based Asynchronous Libraries for Amazon Web Services and clouds that support the AWS APIs
MIT License
32 stars 18 forks source link

client.base BaseQuery/query usage? #77

Open epaulson opened 7 years ago

epaulson commented 7 years ago

I know there's some refactoring of the codebase in progress, but what's the recommended pattern for adding a new service, and specifically, what in client.base should be used?

I'd like to use Step Functions with txaws, and I've been poking around at s3.client, ec2.client, and client.base and they all seem to handle communicating with AWS and generating requests/signatures differently.

I've been able to mostly get a sample request working with just client.base.query and the object that it returns - here's a good chunk of my sample code:

from twisted.web.client import FileBodyProducer
from txaws.client.base import BaseClient, BaseQuery, error_wrapper, RequestDetails, query, url_context
import hashlib
from hashlib import sha256

from txaws import _auth_v4
import txaws.credentials as txcreds

# parts removed
reqbody = "{}"
content_sha256 = sha256(reqbody).hexdigest().decode("ascii")
body_producer = FileBodyProducer(BytesIO(reqbody))
amz_headers = {"Target": "AWSStepFunctions.ListStateMachines"}
headers = Headers({"Content-Type": ["application/x-amz-json-1.0"]})
creds = txcreds.AWSCredentials(mykeyid, mysecret)
urlreq = url_context(scheme=u"https", host=u"states.us-east-1.amazonaws.com",port=None, path=[u""],query=[])
details = RequestDetails(region=b"us-east-1", service=b"states", method=b"POST",url_context = urlreq, amz_headers=amz_headers, headers=headers, body_producer=body_producer, content_sha256=content_sha256)
q = query(credentials=creds, details=details)
d = q.submit()
d.ddCallback(cbRequest)

It doesn't quite work - in _Query in client/base.py, there's a call to _auth_v4._CanonicalRequest.from_request_components() with a hardcoded set of a headers_to_sign - unfortunately for me, I need to add x-amz-target to that list, and I think content-type, but that's not a settable option.

Looking at other services, should I completely build my own query factory and not try to reuse anything, or would it be OK to maybe add a parameter to RequestDetails so I can pass in a list of headers to include in the signed headers list, and build a lightweight wrapper around the base query object? Is there a reason that the base query is intentionally not subclassable and instead hidden behind query()?

Thanks for your advice!

mithrandi commented 7 years ago

You seem to have found the newest iteration of the query code (query / _Query) successfully, so that part is fine.

Subclassing in general is not great; in particular, handling the API layer between base class and subclass is tricky. In this case, I think the correct thing to do is just add x-amz-target to the list in txaws of header fields to sign? However, adding a parameter that you can pass in to customize the list of fields also seems fine; perhaps we should do both — if always signing that header field is correct, then txaws should do presumably do that, but if you're writing downstream code you may not want to wait for a txaws change/release every time you discover a field we should be signing, but are not.

(It seems like signing Content-Type is not required, at least for S3, but I don't understand why not from reading the AWS docs; however, if it were required, then this code would not currently be working for S3)