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: account for null comment #3417

Merged
merged 4 commits into from
Feb 26, 2025
Merged

fix: account for null comment #3417

merged 4 commits into from
Feb 26, 2025

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

Changes

Fixes null comment case in the account resource described in #3402. Because of the limitation in the terraform testing framework, I couldn't create an account externally and then import the created account as a first step of the test (because the testing framework seems to be only working on already existing objects that can be found in-state). That's why I had to test this change manually by using make install-tf and using a locally built provider. I confirm that before accounts created with explicitly null comment COMMENT = null were failing and after applying this change they are not failing anymore.

Copy link

Integration tests cancelled for 5e6c3196b423bf988658052571594807b55b6de4

Copy link
Collaborator

@sfc-gh-jmichalak sfc-gh-jmichalak left a comment

Choose a reason for hiding this comment

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

How about adding a test in resources/manual_tests?

Copy link

Integration tests cancelled for 9d25771071bd57ddc0b121b13973db77ace9c782

Copy link

Integration tests failure for d3ffe6e8c5f9300a9e7078124f2756bfbbdeca51

if err = handleExternalChangesToObjectInShow(d,
outputMapping{"edition", "edition", *account.Edition, *account.Edition, nil},
outputMapping{"is_org_admin", "is_org_admin", *account.IsOrgAdmin, booleanStringFromBool(*account.IsOrgAdmin), nil},
outputMapping{"region_group", "region_group", regionGroup, regionGroup, nil},
outputMapping{"snowflake_region", "region", account.SnowflakeRegion, account.SnowflakeRegion, nil},
outputMapping{"comment", "comment", *account.Comment, *account.Comment, nil},
outputMapping{"comment", "comment", comment, comment, nil},
Copy link
Collaborator

Choose a reason for hiding this comment

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

this line is funny TBH

Copy link
Collaborator Author

@sfc-gh-jcieslak sfc-gh-jcieslak Feb 26, 2025

Choose a reason for hiding this comment

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

Yeah, I think that we should make some adjustments to handle null pointers inside handleExternalChangesToObjectInShow instead of defining non-pointer values before the mappings. It would simplify the codebase for some of the resources.

Copy link

Integration tests failure for 0a58799931eef0185de9e720749f5911b423a72c

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review February 26, 2025 10:32
@sfc-gh-jcieslak sfc-gh-jcieslak merged commit 2f7f80f into dev Feb 26, 2025
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the small-adjustments branch February 26, 2025 10:40
sfc-gh-jcieslak pushed a commit that referenced this pull request Feb 26, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.0.4](v1.0.3...v1.0.4)
(2025-02-26)


### 🔧 **Misc**

* Add GA scope to the roadmap
([#3385](#3385))
([9be2196](9be2196))
* Adjust saml2_integration documentation
([#3415](#3415))
([b8c127d](b8c127d))
* Bump Go version
([#3408](#3408))
([902670e](902670e))
* Bump provider version
([#3419](#3419))
([552e44b](552e44b))
* Remove hardcoded values from documentation
([#3381](#3381))
([cb1d6e2](cb1d6e2))
* Update user docs
([#3416](#3416))
([9d5224f](9d5224f)),
closes
[#3404](#3404)
* Upgrade deps
([#3389](#3389))
([086bf15](086bf15))


### 🐛 **Bug fixes:**

* account for null comment
([#3417](#3417))
([2f7f80f](2f7f80f))
* external function varchar return type validation
([#3400](#3400))
([abf5883](abf5883))
* Fix managed account and adjust account ticket numbers
([#3395](#3395))
([a7cb5cb](a7cb5cb))
* Propose changes to assertions setup with varying test client
([#3406](#3406))
([fb27f6a](fb27f6a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
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.

3 participants