Updated Code samples to AWS SDK 2.x #63

arunx2 · 2022-04-27T22:09:12Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

arunx2 · 2022-07-07T13:12:18Z

Can we merge this PR?

dblock · 2022-07-14T01:58:39Z

Came here to update the same docs!

First, https://github.com/awsdocs/aws-request-signing-apache-interceptor is not maintained, I am working on getting it archived. Evidenced by awsdocs/aws-request-signing-apache-interceptor#1 and awsdocs/aws-request-signing-apache-interceptor#2. It was an attempt at resurrecting the one in awslabs, but didn't get proper ownership.

I think we should stop copy-pasting AwsRequestSigningApacheInterceptor code. The one in the attached zip has a few bugs (e.g. acm19/aws-request-signing-apache-interceptor#38).

There's now a maintained community fork that is AWS SDK 2.0 and has been released to maven: https://github.com/acm19/aws-request-signing-apache-interceptor. I have a demo using that in https://github.com/dblock/opensearch-java-client-demo.

So I propose:

Point to https://github.com/acm19/aws-request-signing-apache-interceptor and update the zip not to include a copy of that code.
Update the doc on compression (you can enable it), and note that chunked transfer encoding currently doesn't work.

We should also (maybe later):

Add an example of using pure AWS SDK v4 to communicate with OpenSearch, e.g. https://github.com/dblock/aws-sdk-sigv4-demo. Useful if you're just trying to make GET requests or send/receive JSON without additional library dependencies.
Start documenting https://github.com/opensearch-project/opensearch-java which has incoming AwsSdk2Transport implementation opensearch-project/opensearch-java#177 for Sigv4 signing / a transport layer that doesn't need an interceptor.

I've also raised to the AWS SDK team the possibility of bringing the fork back under an AWS org, but we don't have to wait for it.

@arunx2 WDYT? I will push this one through into the docs in either state.

lizsnyder · 2022-07-14T03:24:56Z

Hi @arunx2, I'm so sorry that I missed this. For some reason I wasn't receiving notifications of this change, so I didn't see it until someone pointed it out to me today. I will test this out tomorrow to make sure it works, and then merge.

lizsnyder · 2022-07-14T15:32:10Z

@arunx2 do you have any comment on Daniel's comment above?

arunx2 · 2022-07-14T19:04:15Z

@dblock I'll exclude the code and update the zip file. @lizsnyder You can review after the changes with the code.

arunx2 · 2022-07-14T19:11:57Z

@dblock While I like the idea of not copying the code, I'm little skeptical to add a dependency not managed by AWS with an official documentation

io.github.acm19
aws-request-signing-apache-interceptor
2.1.1

I totally agree with your #3 and #4.

dblock · 2022-07-14T20:06:38Z

I think we should reference acm19.* because 1) that's an open-source package just like org.apache.http.* or org.opensearch.action.index.* which aren't managed by AWS either, 2) customers reading the documentation will not be copy-pasting eventually outdated, potentially buggy, or worse, insecure code.

Can we expand the zip in GitHub, so developer can see code changes, and replace the link to the zip to a link to GitHub? A zip is very developer unfriendly, we can't see history, etc.

@lizsnyder Do you have any objections?

lizsnyder · 2022-07-14T20:11:32Z

I think we should reference acm19.* because 1) that's an open-source package just like org.apache.http.* or org.opensearch.action.index.* which aren't managed by AWS either, 2) customers reading the documentation will not be copy-pasting eventually outdated, potentially buggy, or worse, insecure code.

Can we expand the zip in GitHub, so developer can see code changes, and replace the link to the zip to a link to GitHub? A zip is very developer unfriendly, we can't see history, etc.

@lizsnyder Do you have any objections?

Sure, all of that sounds great to me

arunx2 · 2022-07-18T15:27:16Z

@dblock I totally agree with samples in a GitHub repository than the zip file. Do we have GitHub for this sample?

@lizsnyder I've a hesitation to refer this acm19* in official documentation and in my opinion, org.apache.* and org.opensearch.* is backed by a large community and vetted process to get some changes into them. I'm not very familiar with this acm19.* package and not sure if this should be referred in AWS documentation. What's your take?

dblock · 2022-07-18T20:13:26Z

@dblock I totally agree with samples in a GitHub repository than the zip file. Do we have GitHub for this sample?

Can we just use this repo? sample_code/java/amazon-opensearch-docs-sample-client/...? Just not as a zip.

@lizsnyder I've a hesitation to refer this acm19* in official documentation and in my opinion, org.apache.* and org.opensearch.* is backed by a large community and vetted process to get some changes into them. I'm not very familiar with this acm19.* package and not sure if this should be referred in AWS documentation. What's your take?

Note that maven artifacts can't be removed from maven central.

And FWIW, I am a co-maintainer on the acm19 project along with @acm19 who doesn't work for Amazon (I do). If it were to disappear I'd fork it back into opensearch-project at a minimum. The alternative is to copy-paste (aka fork) code. I definitely don't support that, we want users to get bug fixes.

lizsnyder · 2022-07-19T21:52:09Z

Unfortunately I have to agree with @arunx2. It's not an official AWS library, so recommending it would be tough. Looking at this a little more, I think it's best to wait for the AWS SDK team to bring the fork back under an AWS org. Thoughts? Can we expedite that somehow?

dblock · 2022-07-20T17:43:00Z

I don't think we should expect to anything to happen with the AWS SDK team, nor should we wait for it to correct the product documentation that is currently obsolete or has bugs.

@arunx2 want to update this PR with whichever state you think is right based on this discussion and we'll try to see what @lizsnyder is comfortable with merging?

dblock · 2022-07-20T17:44:22Z

doc_source/request-signing.md

        return RestClient.builder(HttpHost.create(host)).setHttpClientConfigCallback(hacb -> hacb.addInterceptorLast(interceptor)).build();
    }
 }
 ```

-If you prefer the high\-level REST client, which offers most of the same features and simpler code, try the following sample, which also uses the [Amazon Web Services Request Signing Interceptor](https://github.com/awslabs/aws-request-signing-apache-interceptor):
+If you prefer the high\-level REST client, which offers most of the same features and simpler code, try the following sample, which also uses the [Amazon Web Services Request Signing Interceptor](https://github.com/awsdocs/aws-request-signing-apache-interceptor):


At the very least this link needs to be removed. The code in that repo is old and should not be used.

lizsnyder · 2022-07-20T18:51:37Z

I don't think we should expect to anything to happen with the AWS SDK team, nor should we wait for it to correct the product documentation that is currently obsolete or has bugs.

@arunx2 want to update this PR with whichever state you think is right based on this discussion and we'll try to see what @lizsnyder is comfortable with merging?

Sounds good to me, thanks.

…x from acm19 repo

arunx2 · 2022-07-26T20:55:59Z

Apologies for the delay. I inherited the bug fix to the sample code and referred source code instead of zip file.

dblock

This looks good enough to me!

I do think we should put SPDX-like copyright headers https://github.com/acm19/aws-request-signing-apache-interceptor/blob/master/src/main/java/io/github/acm19/aws/interceptor/http/AwsRequestSigningApacheInterceptor.java#L1 on all Java files here, but NBD.

lizsnyder · 2022-07-26T21:23:51Z

You guys are awesome! Let me take a day to review/test everything and will hopefully be able to merge tomorrow.

lizsnyder · 2022-07-27T21:56:42Z

Merged. Changes should show up in the public docs in about an hour or two.

arunx2 and others added 3 commits April 26, 2022 18:02

updated java request signing for AWS SDK v2.x

a082db4

updated with new links instead of archieved ones

209ac68

Merge branch 'master' into master

8927ecb

dblock reviewed Jul 20, 2022

View reviewed changes

Merge branch 'awsdocs:master' into master

49fb301

dblock mentioned this pull request Jul 26, 2022

[DOC] AWS Sigv4 Support for Clients opensearch-project/documentation-website#814

Closed

arunx2 added 2 commits July 26, 2022 15:50

Modified to replace the sample zip file with actual code with a bugfi…

6ad9bf4

…x from acm19 repo

moved the java code to a sub directory in 'sample_code'

a695730

dblock approved these changes Jul 26, 2022

View reviewed changes

lizsnyder merged commit 8eb6ca2 into awsdocs:master Jul 27, 2022

This was referenced Sep 9, 2022

class file has wrong version 55.0, should be 52.0 #68

Closed

Current AwsRequestSigningApacheInterceptor removes request content #67

Closed

dblock mentioned this pull request Dec 9, 2022

[FEATURE] Support OpenSearch Serverless request signing opensearch-project/opensearch-java#287

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Code samples to AWS SDK 2.x #63

Updated Code samples to AWS SDK 2.x #63

arunx2 commented Apr 27, 2022

arunx2 commented Jul 7, 2022

dblock commented Jul 14, 2022 •

edited

Loading

lizsnyder commented Jul 14, 2022

lizsnyder commented Jul 14, 2022

arunx2 commented Jul 14, 2022

arunx2 commented Jul 14, 2022

dblock commented Jul 14, 2022 •

edited

Loading

lizsnyder commented Jul 14, 2022

arunx2 commented Jul 18, 2022

dblock commented Jul 18, 2022 •

edited

Loading

lizsnyder commented Jul 19, 2022

dblock commented Jul 20, 2022

dblock Jul 20, 2022

lizsnyder commented Jul 20, 2022

arunx2 commented Jul 26, 2022

dblock left a comment

lizsnyder commented Jul 26, 2022

lizsnyder commented Jul 27, 2022

Updated Code samples to AWS SDK 2.x #63

Updated Code samples to AWS SDK 2.x #63

Conversation

arunx2 commented Apr 27, 2022

arunx2 commented Jul 7, 2022

dblock commented Jul 14, 2022 • edited Loading

lizsnyder commented Jul 14, 2022

lizsnyder commented Jul 14, 2022

arunx2 commented Jul 14, 2022

arunx2 commented Jul 14, 2022

dblock commented Jul 14, 2022 • edited Loading

lizsnyder commented Jul 14, 2022

arunx2 commented Jul 18, 2022

dblock commented Jul 18, 2022 • edited Loading

lizsnyder commented Jul 19, 2022

dblock commented Jul 20, 2022

dblock Jul 20, 2022

Choose a reason for hiding this comment

lizsnyder commented Jul 20, 2022

arunx2 commented Jul 26, 2022

dblock left a comment

Choose a reason for hiding this comment

lizsnyder commented Jul 26, 2022

lizsnyder commented Jul 27, 2022

dblock commented Jul 14, 2022 •

edited

Loading

dblock commented Jul 14, 2022 •

edited

Loading

dblock commented Jul 18, 2022 •

edited

Loading