-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add Route53 ID customization to Botocore. #398
Add Route53 ID customization to Botocore. #398
Conversation
This change moves and adds to the customization [currently in the AWS CLI] (https://github.com/aws/aws-cli/blob/develop/awscli/customizations/route53resourceid.py) that handles automatically splitting Route53 resource IDs. For example, `/hostedzone/ABC123` will be changed to just `ABC123` when used as input to a Route53 service operation. The `Id`, `HostedZoneId`, `ResourceId`, and `DelegationSetId` input parameters are all handled by this customization. This fixes an issue encountered in boto/boto3#28. A corresponding change will need to be made to the AWS CLI to remove the customizations there after this is merged in.
This is handling more cases that what the CLI customization does. Can you verify that: a) these cases are necessary (is there a sample output you can show)? |
It isn't obvious but it's actually handling the same cases. The CLI customization is getting a parameter object which corresponds to the shape of the parameter. The a) For example (this is just one of a handful of methods that need this change): import boto3
route53 = boto3.client('route53')
route53.get_hosted_zone(Id='hostedzone/abcd1234') Before the fix we would get b) The behavior should be backward-compatible, and I have run all existing unit/integ tests and have manually made several calls to Route53 using the updated Botocore and affected parameter names. Everything has worked for me both with the long form ( |
Yeah I'm not saying we don't this change, I'm just confirming that this is an actual port of the CLI customization and won't result in regressions.
Ah ok, that makes sense. I didn't write the original route53 customization, but I'm pretty sure the reason it was done that way is that we won't have to update the customization when a route53 API update decides to use this shape again in a new param name. It seems like with this approach we will, and seems like this is something that will easily be missed. If this is the case, can we figure out a way so we don't have to update the customization as new params are added to the API? |
@jamesls please take another look. I've changed the implementation to use the operation model's input shape members. It now handles all the same cases by just checking the shape names against the two cases in the CLI customization. |
Thanks for updating. |
Add Route53 ID customization to Botocore.
This is now handled by Botocore as of boto/botocore#398.
This change moves and adds to the customization currently in the AWS CLI
that handles automatically splitting Route53 resource IDs.
For example,
/hostedzone/ABC123
will be changed to justABC123
whenused as input to a Route53 service operation. The
Id
,HostedZoneId
,ResourceId
, andDelegationSetId
input parameters are all handled bythis customization. We do not need to handle
HealthCheckId
because theformat is different and as such it doesn't have this issue.
This fixes an issue encountered in boto/boto3#28.
A corresponding change will need to be made to the AWS CLI to remove the
customizations there after this is merged in.
cc @jamesls, @kyleknap