Skip to content

fix(storage): Remove "attribute" concept from UX#1037

Merged
mergify[bot] merged 5 commits intoaws:masterfrom
bvtujo:storage-extra-attributes
Jun 22, 2020
Merged

fix(storage): Remove "attribute" concept from UX#1037
mergify[bot] merged 5 commits intoaws:masterfrom
bvtujo:storage-extra-attributes

Conversation

@bvtujo
Copy link
Copy Markdown
Contributor

@bvtujo bvtujo commented Jun 19, 2020

This PR removes the notion of an Attribute from the UI of copilot storage init.

Previously, customers could specify extra attributes via the --att flag which they could specify for use in Local Secondary Indices. This is problematic, however, because DynamoDB does not allow unused attributes to be written in the CF template and this would cause the addons stack deployment to fail.

Removing "attribute" as a customer facing concept simplifies the user experience by only asking for "alternate sort keys/LSIs" and allows us to avoid complex validation issues which arise when extra attributes are specified.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@bvtujo bvtujo requested a review from a team as a code owner June 19, 2020 21:26
@bvtujo bvtujo self-assigned this Jun 19, 2020
@bvtujo bvtujo requested review from clareliguori, efekarakus and kohidave and removed request for clareliguori June 19, 2020 21:26
Copy link
Copy Markdown
Contributor

@efekarakus efekarakus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just a log statement change

Comment thread internal/pkg/cli/storage_init.go Outdated
}
if !moreAtt {
if len(o.lsiSorts) > 5 {
log.Infof("You may not specify more than 5 alternate sort keys. Continuing...")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be:

Suggested change
log.Infof("You may not specify more than 5 alternate sort keys. Continuing...")
log.Infoln("You may not specify more than 5 alternate sort keys. Continuing...")

@bvtujo bvtujo requested a review from iamhopaul123 June 22, 2020 16:28
Copy link
Copy Markdown
Contributor

@iamhopaul123 iamhopaul123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@mergify mergify bot merged commit ca616f6 into aws:master Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants