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

AWS SDK V2 Upgrade #4939

Closed
wants to merge 26 commits into from
Closed

Conversation

jeevanragula
Copy link
Contributor

@jeevanragula jeevanragula commented Sep 2, 2023

In the Keda project, we are currently using below AWS Go SDK version.

github.com/aws/aws-sdk-go v1.44.287

But AWS released a V2 version long back which has few advantages over V1.
This PR is to update the code to use AWS SDK V2.

Checklist

Fixes #4905

Relates to #4905

Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
@jeevanragula jeevanragula requested a review from a team as a code owner September 2, 2023 17:51
@github-actions
Copy link

github-actions bot commented Sep 2, 2023

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

🏖️ Over the summer, the response time will be longer than usual due to maintainers taking time off so please bear with us.

While you are waiting, make sure to:

Learn more about:

@semgrep-app
Copy link

semgrep-app bot commented Sep 2, 2023

Semgrep found 6 context-todo findings:

  • pkg/scalers/aws_sqs_queue_scaler.go: L232
  • pkg/scalers/aws_kinesis_stream_scaler.go: L177
  • pkg/scalers/aws_dynamodb_streams_scaler.go: L203
  • pkg/scalers/aws_dynamodb_scaler.go: L232
  • pkg/scalers/aws_common.go: L34
  • pkg/scalers/aws_cloudwatch_scaler.go: L370

Consider to use well-defined context

Ignore this finding from context-todo.

Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
@JorTurFer
Copy link
Member

JorTurFer commented Sep 2, 2023

/run-e2e aws
Update: You can check the progress here

Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/metricsservice/api/metrics.pb.go Outdated Show resolved Hide resolved
pkg/metricsservice/api/metrics_grpc.pb.go Outdated Show resolved Hide resolved
pkg/scalers/aws_cloudwatch_scaler.go Show resolved Hide resolved
pkg/scalers/aws_common.go Show resolved Hide resolved
pkg/scalers/aws_kinesis_stream_scaler.go Show resolved Hide resolved
pkg/scalers/aws_sqs_queue_scaler.go Show resolved Hide resolved
pkg/scalers/aws_sqs_queue_scaler.go Show resolved Hide resolved
pkg/scalers/externalscaler/externalscaler.pb.go Outdated Show resolved Hide resolved
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
return attributeValues, nil
}

func attributeValueFromInterface(value interface{}) (types.AttributeValue, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Does the SDK have something like this internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JorTurFer Found this package. But few test cases are failing when using that.

"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"

Copy link
Member

Choose a reason for hiding this comment

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

Do they fail because the behavior has changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TestParseDynamoMetadata test case was failing.
May be I am not using this "attributevalue" package correctly. But with my code, all the test cases are working fine.
If any one suggest easy way to replace this method, happy to do that.

@JorTurFer
Copy link
Member

JorTurFer commented Sep 3, 2023

/run-e2e aws
Update: You can check the progress here

@zroubalik
Copy link
Member

zroubalik commented Sep 5, 2023

/run-e2e aws
Update: You can check the progress here

@JorTurFer
Copy link
Member

JorTurFer commented Sep 5, 2023

/run-e2e aws
Update: You can check the progress here

jeevanragula and others added 4 commits September 7, 2023 00:38
…db_streams_pod_identity_test.go

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
@semgrep-app
Copy link

semgrep-app bot commented Sep 6, 2023

Semgrep found 6 context-todo findings:

  • pkg/scalers/aws_sqs_queue_scaler.go: L241
  • pkg/scalers/aws_kinesis_stream_scaler.go: L177
  • pkg/scalers/aws_dynamodb_streams_scaler.go: L203
  • pkg/scalers/aws_dynamodb_scaler.go: L232
  • pkg/scalers/aws_common.go: L35
  • pkg/scalers/aws_cloudwatch_scaler.go: L371

Consider to use well-defined context

Ignore this finding from context-todo.

Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Wolfe1 and others added 7 commits September 7, 2023 02:13
…#4922)

Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
…ring (kedacore#4584)

* fix RabbitMQ Scaler, allow subpaths along with vhost in connection string (kedacore#4584)

Signed-off-by: Roman Bielyi <romanbielyi.amor@gmail.com>

* Fix CHANGELOG.md, remove redundant/wrong pieces that were made during rebase, this fix is aimed to resolve failed workflow's Static Check (validate-changelog) test

Signed-off-by: Roman Bielyi <romanbielyi.amor@gmail.com>

---------

Signed-off-by: Roman Bielyi <romanbielyi.amor@gmail.com>
Signed-off-by: Jorge Turrado <jorge.turrado@scrm.lidl>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
Signed-off-by: Jeevan Reddy Ragula <jeevanragula@gmail.com>
@jeevanragula
Copy link
Contributor Author

@JorTurFer I have cancelled it and raised a fresh review.
#4953

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.

AWS Go SDK Upgrade
5 participants