Add Amazon Elastic Container Registry (ECR) Hook #28279
Conversation
boring-cyborg
bot
added
area:providers
kind:documentation
provider:amazon
Taragolis
force-pushed
the
amazon-ecr-hook
branch
from
036f1f4 to
1d85ed2
Compare
Taragolis
force-pushed
the
amazon-ecr-hook
branch
from
1d85ed2 to
8208edd
Compare
o-nikolas
left a comment
o-nikolas
left a comment
There was a problem hiding this comment.
Just a nit and a question, otherwise looks good!
There was a problem hiding this comment.
Is this just to guard empty string/list as an input? Because you are already defaulting this to None in the method signature.
There was a problem hiding this comment.
Yep, just a guard. This attribute will propagated from connection
There was a problem hiding this comment.
So value in some cases might be "" just for avoid this situation.
There was a problem hiding this comment.
@o-nikolas @ferruzzi do we have any concern about this part? Or we could merge this changes?
There was a problem hiding this comment.
Yeah no concern I was just checking to be sure. You could re-arrange the if statements so that the falsey check happens first so that you wouldn't need this assignment, but the way you have it is also perfectly fine 👍
Co-authored-by: D. Ferruzzi <ferruzzi@amazon.com> Co-authored-by: Niko <onikolas@amazon.com>
Taragolis
force-pushed
the
amazon-ecr-hook
branch
from
da72aec to
7fb732d
Compare
Co-authored-by: D. Ferruzzi <ferruzzi@amazon.com> Co-authored-by: Niko <onikolas@amazon.com>
5 participants
Pre-requirements for #26162
Add new Hook into amazon-provider which integrated with ECR.
Hook itself it is just Thin wrapper around boto3 ECR client with single method for:
docker login