Add @auth trait breaking change rules
#2252
Conversation
| Adding the `@auth` trait will explicitly define the order of the \ | ||
| authentication scheme traits for the target shape. \ |
There was a problem hiding this comment.
Previous comment thread: #2245 (comment)
From this message it is unclear why explicitly defining the order might be a danger. Probably talk about the default like in the remove case below.
These messages seem to be geared towards theserviceshape. Seems like they'd need to be slightly different for theoperationshape. You only have tests for the service shape cases. Maybe having operation shape cases would allow us to see if the error messages are helpful enough for the case.
I also wonder if we should instead do this with the getEffectiveAuthSchemes approach to eliminate noise where there is no effective change. Like you have in the Migration classes, but generically, with sigv4 cases being special cases?
I agree, the validation for@authshould be more comprehensive withServiceIndex::getEffectiveAuthSchemesfor both service and operation levels.
| change: "remove" | ||
| severity: "DANGER" | ||
| message: """ | ||
| Removing the `@auth` trait will default to using the alphabetical \ |
There was a problem hiding this comment.
Previous comment thread: #2245 (comment)
If removed from operation, will default to using the service level
@authif present. I think the emphasis again is on the risk that the effective auth schemes may change.
Background
Testing
Links
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.