> This change looks good to me at first glance (reading the diff on the merge
> proposal; I haven't branched the code yet). In particular, the amz_headers
> functionality is much-needed.
>
> Drew, I don't see anything about the new web client usage, only headers and
> body producer. Is that coming in a future change?
>
Ok, so I had to remember myself while staring at the diff. The code for agent is in the prerequisite branch: lp:~djfroofy/txaws/modernize-924459
> At the risk of being a total pain in the ass, here's what I would recommend:
>
No, no worries.
> 1) split out the headers changes into a new branch, and attach it to bug
> #972432.
#972432 must be a duplicate of #640098; the amz_headers argument has been merged already. To summarize what this diff is adding: Support for plugging in Body Producers and Receivers in s3.client (this is made possible by: lp:~djfroofy/txaws/modernize-924459).
There are actually no changes related to headers in this patch. Rather, I'm adding receiver_factory as __init__() argument to S3Client and body_producer as argument to regular S3 api calls.
Cutting to the point, I think #972432 needs to be reviewed first.
> 2) create a new bug for the body producer/receiver factory feature, split that
> code out, and attach it to there in its own branch.
That was in the scope of #972432
> 3) do the work on the new agent in this branch and this ticket (bug #767205).
> This change looks good to me at first glance (reading the diff on the merge
> proposal; I haven't branched the code yet). In particular, the amz_headers
> functionality is much-needed.
>
> Drew, I don't see anything about the new web client usage, only headers and
> body producer. Is that coming in a future change?
>
Ok, so I had to remember myself while staring at the diff. The code for agent is in the prerequisite branch: lp:~djfroofy/txaws/modernize-924459
> At the risk of being a total pain in the ass, here's what I would recommend:
>
No, no worries.
> 1) split out the headers changes into a new branch, and attach it to bug
> #972432.
#972432 must be a duplicate of #640098; the amz_headers argument has been merged already. To summarize what this diff is adding: Support for plugging in Body Producers and Receivers in s3.client (this is made possible by: lp:~djfroofy/txaws/modernize-924459).
There are actually no changes related to headers in this patch. Rather, I'm adding receiver_factory as __init__() argument to S3Client and body_producer as argument to regular S3 api calls.
Cutting to the point, I think #972432 needs to be reviewed first.
> 2) create a new bug for the body producer/receiver factory feature, split that
> code out, and attach it to there in its own branch.
That was in the scope of #972432
> 3) do the work on the new agent in this branch and this ticket (bug #767205).
Again: #972432 is where that functionality is.