DanielLeens commented on PR #11423: URL: https://github.com/apache/seatunnel/pull/11423#issuecomment-5385673307
Thanks @SEZ9 for the additional pass. The head is unchanged from the one I approved on August 4 (`b7b7288ad234`), so I'm treating this as engaging with your seven follow-up points rather than reopening the review. All seven read as reasonable, non-blocking refinements to the container-credential guidance rather than anything factually wrong in the current text: the ECS `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` prerequisite (Issue 1), naming the actual bundled SDK artifact/version boundary instead of an unqualified IRSA claim (Issue 3), and the IMDS blast-radius note for EKS node-role credentials (Issue 4) are all worth folding in — they sharpen guidance that is already directionally correct. I'd put Issues 2, 5, 6, and 7 in the same bucket: real polish on wording, cross-referencing, and formatting, but none of them change what a reader would actually configure. From Daniel's side, none of these rise to a blocker on top of the docs-only changes already verified in this PR (the `s3a://` scheme fix, the corrected provider class name, and the SDK-boundary facts checked against the pinned `aws-java-sdk-bundle:1.11.271`), so my August 4 approval stands. It would be great to see a small follow-up commit picking up these seven points before merge, but I wouldn't hold the PR open only for them. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
