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

private/model/api: Generating useful examples #1309

Closed
wants to merge 4 commits into from
Closed

Conversation

xibz
Copy link
Contributor

@xibz xibz commented May 30, 2017

No description provided.

@xibz xibz requested a review from jasdel May 30, 2017 23:41
@xibz xibz force-pushed the ex branch 4 times, most recently from 83050f9 to ea62fbd Compare May 31, 2017 05:51
Copy link
Contributor

@jasdel jasdel left a comment

Choose a reason for hiding this comment

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

I think there is an issue with the generator not generating dynamodb.AttributeValues correctly. Looked at a few examples an noticed none of the nested structures were being generated. The example models include values though.

I didn't look at the other services to see if they had similar issues.

https://github.com/aws/aws-sdk-go/blob/master/models/apis/dynamodb/2012-08-10/examples-1.json#L75-L90


switch v := shape.(type) {
case bool:
//return fmt.Sprintf("%s: aws.Bool(%t),\n", memName, v)
Copy link
Contributor

Choose a reason for hiding this comment

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

these comments still needed?

return parseTimeString(ref, memName, fmt.Sprintf("%d", v))
} else {
return convertToCorrectType(memName, ref.Shape.MemberRefs[name].Shape.Type, fmt.Sprintf("%d", v))
//return fmt.Sprintf("%s: aws.Int64(%d),\n", memName, v)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

}
}

func buildComplex(name, memName string, ref *ShapeRef, v map[string]interface{}) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed the buildComplex doesn't handle slices is that just because we didn't have any list data in example model?

memName := name
if isMap {
memName = fmt.Sprintf("%q", memName)
} else if ref != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

empty conditional


// buildShape will recursively build the referenced shape based on the json object
// provided.
func buildShape(ref *ShapeRef, shapes map[string]interface{}, isMap bool) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be helpful to document the isMap parameter and what its expectations are

@@ -9133,7 +9133,7 @@ type Group struct {
HealthCheckType *string `min:"1" type:"string" required:"true"`

// The EC2 instances associated with the group.
Instances []*Instance `type:"list"`
Instances []*InstanceDetails `type:"list"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this file should of changed.

if v := len(v); v != 0 {
t.Errorf("expect no values, got %d", v)
if l := len(v); l != 0 {
t.Errorf("expect no values, got %d", l)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where these changes to this file intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, linter was complaining about v being shadowed

func ExampleDynamoDB_BatchWriteItem_shared00() {
svc := dynamodb.New(session.New())
input := &dynamodb.BatchWriteItemInput{
RequestItems: map[string][]*dynamodb.WriteRequest{},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are missing nested values from the example model. Based on the description it sounds like there should be additional fields.

input := &dynamodb.DeleteItemInput{
Key: map[string]*dynamodb.AttributeValue{
"Artist": {},
"SongTitle": {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these have fields's AttributeValue have values?

Item: map[string]*dynamodb.AttributeValue{
"AlbumTitle": {},
"Artist": {},
"SongTitle": {},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think something in the example generator is preventing AttributeValue fields from getting rendered. Or is these just missing from the model?

@xibz xibz force-pushed the ex branch 5 times, most recently from 6e45f8d to 6e12fba Compare June 1, 2017 19:22
@xibz xibz force-pushed the ex branch 2 times, most recently from 4d800e3 to c44370c Compare June 2, 2017 21:14
@xibz xibz closed this Jun 2, 2017
@xibz xibz deleted the ex branch June 22, 2017 20:12
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.

None yet

2 participants