Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(v2): document use of aws-crt-client (#1092) #1605

Merged

Conversation

jreijn
Copy link
Contributor

@jreijn jreijn commented Mar 14, 2024

Issue #, if available: #1092

Description of changes:

  • Added documentation for using the aws-crt-client with powertools (v2).
  • Fixed some documentation issues with typos and code blocks.

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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

docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated
```

After configuring the dependencies it's required to specify the aws sdk http client.
Most modules support a custom sdk client by leveraging the `.withClient()` method on the for instance the Provider singleton:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something missing here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to clarify it a bit more. I noticed I focussed too much on the parameters module, but other modules also use the url-connection client. Turned it into an example for a module and described that it depends on the PT module in use how to further configure the http client.

jreijn and others added 4 commits March 17, 2024 14:13
Co-authored-by: Jérôme Van Der Linden <117538+jeromevdl@users.noreply.github.com>
Co-authored-by: Jérôme Van Der Linden <117538+jeromevdl@users.noreply.github.com>
Co-authored-by: Jérôme Van Der Linden <117538+jeromevdl@users.noreply.github.com>
@jreijn
Copy link
Contributor Author

jreijn commented Mar 17, 2024

@jeromevdl thank you for the thorough review! I've incorporated most of your suggestions and tried to be more explicit on the unclear part.

Copy link
Contributor

@scottgerring scottgerring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Couple of nitpicks inline but looking good. Also immediately helpful for the next time I need to remember how to set this all up :D

docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
docs/FAQs.md Outdated Show resolved Hide resolved
jreijn and others added 8 commits March 18, 2024 14:38
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
@jreijn
Copy link
Contributor Author

jreijn commented Mar 18, 2024

Some great additions @scottgerring ! Thanks so much. I do have one outstanding issue for myself. I mentioned the <version>1.18.0</version> as the version for the dependency. As this is going into the v2 branch should I replace that with 2.x.x? As the code example is for the v2 branch.

@scottgerring
Copy link
Contributor

Some great additions @scottgerring ! Thanks so much. I do have one outstanding issue for myself. I mentioned the <version>1.18.0</version> as the version for the dependency. As this is going into the v2 branch should I replace that with 2.x.x? As the code example is for the v2 branch.

If you use 2.0.0 we should be safe! We've got a rather rough script in the build that goes through and subs this all out when we do releases. I believe we're using 2.0.0 elsewhere in the docs.

Copy link

sonarcloud bot commented Mar 18, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@scottgerring scottgerring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Cheers @jreijn

@scottgerring scottgerring merged commit e4e2cd1 into aws-powertools:v2 Mar 21, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants