Skip to content

Conversation

sirutBuasai
Copy link
Member

Issue #, if available:

Description of changes:

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

@sirutBuasai sirutBuasai marked this pull request as ready for review October 9, 2025 16:43
Copy link

ack-prow bot commented Oct 9, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ack-prow ack-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 9, 2025
@sirutBuasai sirutBuasai changed the title chore: Upgrade aws-go-sdk-v2 from 1.32 to 1.39 chore: Upgrade aws-sdk-go-v2 from 1.32 to 1.39 Oct 9, 2025
Copy link
Member

@michaelhtm michaelhtm left a comment

Choose a reason for hiding this comment

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

Thanks @sirutBuasai 👍
left a few comments below

Copy link
Member

@michaelhtm michaelhtm left a comment

Choose a reason for hiding this comment

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

I left a few more comments regarding some new fields that we're adding with the new sdk version. I just wanted to double check that we're making sure all new spec fields that we support, we also support updates for them when possible

Comment on lines +925 to +932
if r.ko.Spec.DomainSettings.TrustedIdentityPropagationSettings != nil {
f5f4 := &svcsdktypes.TrustedIdentityPropagationSettings{}
if r.ko.Spec.DomainSettings.TrustedIdentityPropagationSettings.Status != nil {
f5f4.Status = svcsdktypes.FeatureStatus(*r.ko.Spec.DomainSettings.TrustedIdentityPropagationSettings.Status)
}
f5.TrustedIdentityPropagationSettings = f5f4
}
if r.ko.Spec.DomainSettings.UnifiedStudioSettings != nil {
Copy link
Member

Choose a reason for hiding this comment

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

does this also not support updates?

Copy link
Member Author

Choose a reason for hiding this comment

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

Domain does have an update ops and these settings are part of update ops also

}
f5elemf9.InstanceGroups = f5elemf9f1
}
if f5iter.ResourceConfig.InstancePlacementConfig != nil {
Copy link
Member

Choose a reason for hiding this comment

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

does this not support updates either?

Copy link
Member Author

Choose a reason for hiding this comment

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

HyperParameterTuningJob doesn't have update

Comment on lines +1795 to +1811
if f5elemf7iter.DataSource.S3DataSource.HubAccessConfig != nil {
f5elemf7elemf3f1f1 := &svcsdktypes.HubAccessConfig{}
if f5elemf7iter.DataSource.S3DataSource.HubAccessConfig.HubContentARN != nil {
f5elemf7elemf3f1f1.HubContentArn = f5elemf7iter.DataSource.S3DataSource.HubAccessConfig.HubContentARN
}
f5elemf7elemf3f1.HubAccessConfig = f5elemf7elemf3f1f1
}
if f5elemf7iter.DataSource.S3DataSource.InstanceGroupNames != nil {
f5elemf7elemf3f1.InstanceGroupNames = aws.ToStringSlice(f5elemf7iter.DataSource.S3DataSource.InstanceGroupNames)
}
if f5elemf7iter.DataSource.S3DataSource.ModelAccessConfig != nil {
f5elemf7elemf3f1f3 := &svcsdktypes.ModelAccessConfig{}
if f5elemf7iter.DataSource.S3DataSource.ModelAccessConfig.AcceptEula != nil {
f5elemf7elemf3f1f3.AcceptEula = f5elemf7iter.DataSource.S3DataSource.ModelAccessConfig.AcceptEula
}
f5elemf7elemf3f1.ModelAccessConfig = f5elemf7elemf3f1f3
}
Copy link
Member

Choose a reason for hiding this comment

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

same here`

Copy link
Member Author

Choose a reason for hiding this comment

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

same here, this only has create/delete ops

@michaelhtm
Copy link
Member

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 10, 2025
Copy link

ack-prow bot commented Oct 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michaelhtm, sirutBuasai

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-prow ack-prow bot added the approved label Oct 10, 2025
@ack-prow ack-prow bot merged commit 060190f into aws-controllers-k8s:main Oct 10, 2025
7 checks passed
@sirutBuasai sirutBuasai deleted the sdk-upgrade branch October 10, 2025 19:49
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.

2 participants