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

fix(vpc): add data annotation to vpc #4534

Merged
merged 1 commit into from
Sep 22, 2023
Merged

Conversation

dmart
Copy link
Contributor

@dmart dmart commented Sep 22, 2023

Annotate VPC as a data class to address VPC attributes always being null.

@dbyron-sf
Copy link
Contributor

Can you tell us more about what fails without this? Possible to add a test that demonstrates it?

@dmart
Copy link
Contributor Author

dmart commented Sep 22, 2023

Calls to findForRegionAndAccount throw an NPE, without a constructor or getters/setters Jackson is unable to set the properties. This worked previously before being ported to Java.

@dbyron-sf
Copy link
Contributor

OK, thanks. For the record, that conversion to java happened in #4269. I'm all for fixing this, but would love a test to keep us from breaking this again if you're up for it.

@dmart
Copy link
Contributor Author

dmart commented Sep 22, 2023

Added a simple test.

Annotate VPC as a data class to address VPC attributes always being null.
Copy link
Contributor

@dbyron-sf dbyron-sf left a comment

Choose a reason for hiding this comment

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

Thank you!

@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Sep 22, 2023
@mergify mergify bot added the auto merged Merged automatically by a bot label Sep 22, 2023
@mergify mergify bot merged commit 9a1fa51 into spinnaker:master Sep 22, 2023
4 checks passed
@dbyron-sf
Copy link
Contributor

#4269 first went in to 1.29, which is old enough that we're not releasing fixes for it. Gonna backport the fix there, and to the other live branches.

@dbyron-sf
Copy link
Contributor

@Mergifyio backport release-1.29.x release-1.30.x release-1.31.x release-1.32.x

@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2023

backport release-1.29.x release-1.30.x release-1.31.x release-1.32.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)
mergify bot pushed a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)
mergify bot pushed a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)
mergify bot pushed a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)
mergify bot added a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)

Co-authored-by: David Martin <david.martin@smartthings.com>
mergify bot added a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)

Co-authored-by: David Martin <david.martin@smartthings.com>
mergify bot added a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)

Co-authored-by: David Martin <david.martin@smartthings.com>
mergify bot added a commit that referenced this pull request Sep 22, 2023
Annotate VPC as a data class to address VPC attributes always being null.

(cherry picked from commit 9a1fa51)

Co-authored-by: David Martin <david.martin@smartthings.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge target-release/1.33
Projects
None yet
3 participants