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

Update ElastiCache CodeGen test based on new SDK #75

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

nmvk
Copy link
Member

@nmvk nmvk commented Jun 2, 2021

ElastiCache would use new AWS SDK Go to include new features like LogDelivery which are currently not part of elasticache-controller. This PR updates the test data used in code generator for Elasticache tests.

Additionally I have updated generator.yaml to be inline with one being used in elasticache-controller. Updated unit tests which use CacheCluster resource to ReplicationGroup.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jaypipes
Copy link
Collaborator

jaypipes commented Jun 2, 2021

@nmvk no changes to the sdk.go generation?

@a-hilaly
Copy link
Member

a-hilaly commented Jun 2, 2021

@nmvk what changes does this version bring exactly? This repository is using aws/aws-sdk-go for it private/model package.
If you need this version for elasticache-controller code generation you can set it using AWS_SDK_GO_VERSION env variable

Update json files in test to use v1.38.52.

Used ReplicationGroup for some old tests and
updated generator.yaml to potentially match
elasticache.
@nmvk nmvk changed the title Update SDK to 1.38.52 Update ElastiCache CodeGen test based on new SDK Jun 3, 2021
@a-hilaly
Copy link
Member

a-hilaly commented Jun 3, 2021

/test all

@ack-bot
Copy link
Collaborator

ack-bot commented Jun 3, 2021

@a-hilaly: No jobs can be run with /test all.
The following commands are available to trigger jobs:

  • /test unit-test

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@a-hilaly
Copy link
Member

a-hilaly commented Jun 3, 2021

/test unit-test

@nmvk
Copy link
Member Author

nmvk commented Jun 3, 2021

Thanks @jaypipes @a-hilaly I got confused about the usage of the aws-sdk-go in this project. I originally thought our tests used them. I have updated the PR with ElastiCache specific change.

@a-hilaly
Copy link
Member

a-hilaly commented Jun 3, 2021

Thanks! Yes i agree that it is a bit confusing :)

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

👍

@a-hilaly
Copy link
Member

a-hilaly commented Jun 3, 2021

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Jun 3, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, nmvk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 1745951 into aws-controllers-k8s:main Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants