-
Notifications
You must be signed in to change notification settings - Fork 153
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
upgrade dependency aws-sdk-v2 #143
Conversation
Nice work on this @plopezlpz! I wonder if it would be preferred to maintain separate v1 and v2 Go modules, similar to the Java implementations? Likely requires breaking/refactoring the current Go module, but it may be cleaner for any users who are bound to v1. Hey @IanMeyers, is there any chance this PR can get eyes on it soon (and your input on the above)? I'm preparing a contribution for another open source repo that adds Go deaggregation support, and it already uses Thanks! |
would be great to have a versioned directory! nice work |
go/deaggregator/deaggregator.go
Outdated
@@ -22,9 +22,9 @@ const ( | |||
|
|||
// DeaggregateRecords takes an array of Kinesis records and expands any Protobuf | |||
// records within that array, returning an array of all records | |||
func DeaggregateRecords(records []*kinesis.Record) ([]*kinesis.Record, error) { | |||
func DeaggregateRecords(records []*types.Record) ([]*types.Record, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thoughts on updating this to use []types.Record
? The v2-sdk doesn't return a slice of pointers anymore which means any consumer of this library is going to need to jump through some hoops to create a new slice of pointers to pass to this func
// Represents the output for GetRecords.
type GetRecordsOutput struct {
// The data records retrieved from the shard.
//
// This member is required.
Records []types.Record
|
||
| SDK | Project | | ||
| --- | ------- | | ||
|Version 0 | [v0](.) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the first version referred to as v0 or v1? I'd assume v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when importing it it is github.com/awslabs/kinesis-aggregation/go v0.0.0-20210630091500-54e17340d32f
so I left it as v0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. so versioning it based on THIS repo vs the version of AWS SDK it's being consumed from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems pretty well to me
There are a couple of libraries it can't upgrade to aws-go-sdk-v2 because of this package. could someone do merge? |
… it. awslabs/kinesis-aggregation#143 (comment) Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com>
* use cammel case Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * fix credential usage Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * apply lint against MD file Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * upgrade versions Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * remove calls to deprecated functions. Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * fix cammel case usage Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * since deaggregation package was upgraded to sdk v2 makes sense to use it. awslabs/kinesis-aggregation#143 (comment) Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> * fix format Signed-off-by: Fabiano Graças <fabiano.gracas@faro.com> Co-authored-by: Fabiano Graças <fabiano.gracas@faro.com>
Issue #, if available:
Description of changes:
Upgrade dependency to use aws-sdk-v2
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.