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

Java v2 Add S3 Express Scenario #7270

Merged
merged 10 commits into from
Mar 24, 2025
Merged

Java v2 Add S3 Express Scenario #7270

merged 10 commits into from
Mar 24, 2025

Conversation

scmacdon
Copy link
Contributor

@scmacdon scmacdon commented Mar 3, 2025

This pull request adds Java S3 Express Scenario


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@scmacdon scmacdon added Java-v2 This issue relates to the AWS SDK for Java V2 Basics A basic code example showing the core actions for a particular service. labels Mar 3, 2025
@scmacdon scmacdon self-assigned this Mar 3, 2025
@scmacdon scmacdon requested a review from tkhill-AWS March 5, 2025 19:41
@tkhill-AWS
Copy link
Collaborator

tkhill-AWS commented Mar 14, 2025

Overall comments

Always refer to "regular" buckets rather than "normal" buckets. Lowercase the "d" in "Directory buckets".

README changes

  • The README for this S3Express Basics Scenario has the wrong title. It reads "Amazon S3 Batch Basics Scenario".
  • Under the [Setting up Resources](Amazon S3 Batch Basics Scenario) section, it says an IAM role will be created by a CloudFormation template, but this is not accurate for this Java implementation.

Specification changes

  • In the Overview section, change "...using the AWS SDK." to "...using an AWS SDK." Lowercase "Directory" in "Directory bucket".

  • In the Resources and User Input section, "{bunch of stuff}" should be replaced.

  • Under Scenario Program Flow,

    • First item - I don't think we should do the VPC endpoint, unless we plan to create a runnable jar with instructions on how to do get that up and running on an EC2 instance. If we do go with it, I think what needs to happen is the VPC endpoint needs to be created in whatever VPC the EC2 instance is currently running it, because if you create a new VPC, the current EC2 instance won't have launched in it. You'd have to relaunch the EC2 instance into the new VPC.
    • Second item - Don't create IAM users with long-lived credentials. Just create roles and assume them. This example should use 2 roles with different permssions rather than creating two IAM users with long-lived credentials. This is a best practice, so this is what we should show users.

I'm still reviewing ....

Copy link
Collaborator

@tkhill-AWS tkhill-AWS left a comment

Choose a reason for hiding this comment

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

I've done as much review as I have time for. Check out the comment with the "Overall Comments". It contains comments related to the README and Specification.

Copy link
Collaborator

@tkhill-AWS tkhill-AWS left a comment

Choose a reason for hiding this comment

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

Scott and I went through the changes in our weekly meeting. Scott will send my concers regarding the VPC issue and IAM user creation to Jason.

@scmacdon scmacdon added the On Call Review needed This work needs an on-call review label Mar 19, 2025
@scmacdon
Copy link
Contributor Author

I sent the feedback about Spec, etc to Jason.

@scmacdon scmacdon added On Call Review needed This work needs an on-call review and removed On Call Review needed This work needs an on-call review labels Mar 19, 2025
@beqqrry-aws beqqrry-aws added On Call Review complete On call review complete and removed On Call Review needed This work needs an on-call review labels Mar 24, 2025
@beqqrry-aws beqqrry-aws merged commit e2713b2 into awsdocs:main Mar 24, 2025
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Basics A basic code example showing the core actions for a particular service. Java-v2 This issue relates to the AWS SDK for Java V2 On Call Review complete On call review complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants