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

Refactoring API client package layout to group enums, types in their own respective packages. #445

Closed
skotambkar opened this issue Dec 2, 2019 · 0 comments
Labels
wontfix We have determined that we will not resolve the issue.

Comments

@skotambkar
Copy link
Contributor

skotambkar commented Dec 2, 2019

This issue details how the v2 AWS SDK for Go's API client packages can be updated as a step towards improving generated serializers performance. The proposed changes in this design seek to improve the v2 SDK package layout to group service enums, types in their own respective packages.

We'd like to hear your feedback on this proposed refactor to the v2 SDK package layout.

Motivation

We aim to improve the generated serializers performance for the AWS SDK for Go. A step towards this goal is to create stand-alone operation serializer functions and reduce use of reflection/ run-time assertions in the SDK. This would require us to refactor the Service client package and group enums, types in their own respective packages to avoid cyclic dependencies in the Go SDK.

Currently, the Go SDK's API Client Package is cluttered with client operations, types, enums, errors and client specific customizations. The proposed change of moving the types, enums in their own respective packages would also help improve their discoverability.

Proposed Changes

We propose a refactor to the SDK's API Client package layout. The changes proposes splitting the API Client package in three primary packages with canonical dependency.

For example for s3 service, the three packages would be :

  • Package s3
  • Package types
  • Package enums

The dependency flow would be as follows:

Given a set of objects S and a transitive relation R ⊆ S × S with ( a , b ) ∈ R modeling a dependency "a depends on b"
 S = { s3 pkg , types pkg , enums pkg}
 R = {(s3 pkg , types pkg), (s3 pkg , enums pkg), (types pkg , enums pkg)}

  • s3 package will depend on both types and enums packages
  • types package might depend on enums packages.

The enums package would not depend on any of the above two packages. This packaging layout helps avoid circular reference

The proposed refactor of the SDK's API Client package would create a package layout as follows :

|-service/
	|- s3/
		|- api_client.go
		|- ap_op_opname.go	
		|- enums
			|- api_enums.go
		|- types
			|- api_op_opname.go
				- operation input
				- operation output
			|- api_types.go 			
		|- <Others>

Usage example

The following example invokes the Amazon S3 API Client's Create Bucket Request using the new types, enums package.

// To create a bucket in a specific region
//
// The following example creates a bucket. The request specifies an AWS region
// where to create the bucket.
package main 

import (
	"context"
	"fmt"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/aws/awserr"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/enums"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"
)

func ExampleClient_CreateBucketRequest() {
	cfg, err := external.LoadDefaultAWSConfig()
	if err != nil {
		panic("failed to load config, " + err.Error())
	}

	svc := s3.New(cfg)
	input := &types.CreateBucketInput{
		Bucket: aws.String("examplebucket"),
		CreateBucketConfiguration: &types.CreateBucketConfiguration{
			LocationConstraint: enums.BucketLocationConstraintEuWest1,
		},
	}

	req := svc.CreateBucketRequest(input)
	result, err := req.Send(context.Background())
	if err != nil {
		if aerr, ok := err.(awserr.Error); ok {
			switch aerr.Code() {
			case s3.ErrCodeBucketAlreadyExists:
				fmt.Println(s3.ErrCodeBucketAlreadyExists, aerr.Error())
			case s3.ErrCodeBucketAlreadyOwnedByYou:
				fmt.Println(s3.ErrCodeBucketAlreadyOwnedByYou, aerr.Error())
			default:
				fmt.Println(aerr.Error())
			}
		} else {
			// Print the error, cast err to awserr.Error to get the Code and
			// Message from an error.
			fmt.Println(err.Error())
		}
		return
	}

	fmt.Println(result)
}

We have created a PR #441 that includes this change.

@skotambkar skotambkar added breaking-change Issue requires a breaking change to remediate. refactor labels Dec 2, 2019
@skotambkar skotambkar added wontfix We have determined that we will not resolve the issue. and removed breaking-change Issue requires a breaking change to remediate. refactor labels Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix We have determined that we will not resolve the issue.
Projects
None yet
Development

No branches or pull requests

1 participant