baolsen commented on pull request #16771: URL: https://github.com/apache/airflow/pull/16771#issuecomment-874478743
> With the one nitpick request above (which I won't make a required change), and the caveat that I still have a bit to learn about the AWS auth/credential system, I think it looks good. Seems like a good addition and I don't see anything jumping out at me as an obvious issue. Thanks. My only "concern" is that I can't verify that the assume_role_with_web_identity control path works on my environment. I guess that is what the unit tests are for, but it would be handy if someone who uses that control path could pull this and verify it works for them. Logically I have stepped through the changes and it does look correct. > [EDIT] Second nitpick, I believe the three commits need to be squashed, or perhaps I misunderstood when I read that somewhere. Kamil or Jarek can confirm or refute that. I'm also not sure... I figured that separate commits may be easier to review what changed, provided it's not too spammy. Happy to rebase and squash if the others ask, otherwise I guess it can be done on merge :) -- 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]
