Skip to content
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 MaxItems attribute to Schema #5218

Merged

Conversation

vancluever
Copy link
Contributor

Hey guys,

This PR adds a new field for use in a resource Schema: MaxItems.

From the commit log:

 * MaxItems defines a maximum amount of items that can exist within a
   TypeSet or TypeList. Specific use cases would be if a TypeSet is being
   used to wrap a complex structure, however more than one instance would
   cause instability.

Of course, this can be used to limit the size of plain old lists, or more general-use sets, just in case a request size does need to be limited for some reason.

I created this to support my new aws_cloudfront_distribution resource, which has several complex structures in it that will behave badly if you specify more than one instance of a structure that is only expected to have a single element. See comments on #3330. I have tested this with the acceptance tests on my CloudFront branch, and also have included unit tests for the functionality itself.

The plan being once this is accepted I will add this to the aws_cloudfront_distribution resource appropriately.

// Validate length
if schema.MaxItems > 0 && rawV.Len() > schema.MaxItems {
return nil, []error{fmt.Errorf(
"%s: exceeded MaxItems of %d", k, schema.MaxItems)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this error message will end up being user facing, let's make the language verbose so the user understands exactly what they need to fix.

Something like (w/ an in-context example):

aws_instance.foo: root_block_device: attribute supports 1 item maximum, config has 2 declared

Maybe we can think of some clearer language, but just something to help users correct their config when they see the error is what I'm getting at.

@phinze
Copy link
Contributor

phinze commented Feb 23, 2016

This is looking good! Let's get the error message figured out and then this will be good to go. 👌

 * MaxItems defines a maximum amount of items that can exist within a
   TypeSet or TypeList. Specific use cases would be if a TypeSet is being
   used to wrap a complex structure, however more than one instance would
   cause instability.
@vancluever
Copy link
Contributor Author

attribute supports 1 item maximum, config has 2 declared

Sounds good enough to me. :) Updated and squashed!

@apparentlymart
Copy link
Contributor

Looking good. Thanks!

apparentlymart added a commit that referenced this pull request Mar 1, 2016
@apparentlymart apparentlymart merged commit c1ce8ff into hashicorp:master Mar 1, 2016
@vancluever
Copy link
Contributor Author

Thanks @apparentlymart! Now I can add this to #5221.

@ghost
Copy link

ghost commented Apr 27, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants