Skip to content

Fixed handling of zero values in retention attributes.#162

Merged
jtaylor-cs merged 2 commits intomasterfrom
CENG-590_Retention_Attributes_Recognize_Zero_Values
Dec 11, 2025
Merged

Fixed handling of zero values in retention attributes.#162
jtaylor-cs merged 2 commits intomasterfrom
CENG-590_Retention_Attributes_Recognize_Zero_Values

Conversation

@jtaylor-cs
Copy link
Contributor

@jtaylor-cs jtaylor-cs commented Nov 26, 2025

Fixes the treatment of a zero value for the attributes retention_count_limit and retention_day_limit, in which the zero value was overridden by the default value of the attribute.

@jtaylor-cs jtaylor-cs requested a review from a team as a code owner November 26, 2025 13:45
Copilot AI review requested due to automatic review settings November 26, 2025 13:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the handling of zero values in the Terraform retention rule attributes, retention_count_limit and retention_day_limit. A zero value set of these attributes resulted in the value being overwritten with its default value:

retention_count_limit = 100
retention_day_limit = 28

Expectations:

Following the upgrade of the provider, a user may notice that a Terraform Plan/Apply action reflects changes to these limit attribute values. This occurs because the zero value is now recognized and is correcting the incorrectly set default value.

Key Changes:

  • Modified the update logic to always send retention_count_limit, retention_days_limit, and retention_size_limit values to the API, even when zero.
  • Updated test case to validate that zero values for retention limits persist correctly.
  • Updated documentation for retention_count_limit to include default value of 100.
  • Refined documentation for retention_size_limit attribute to better reflect the maximum 20 GB limit and equivalent byte value.

@BartoszBlizniak
Copy link
Member

Hey @jtaylor-cs -

  • Just to double-check, will these changes have any impact on the current users who use retention rules via Terraform? You should be able to perform a quick check by; running the code once on the latest version available (0.0.66) and then switching to your build to see if there are any differences which may come up as a result - if this will start spitting out differences for existing users, we should document this somewhere.
  • Question (optional); is there any API changes which we can suggest to our team that would make this experience better? Whilst this will fix stuff in short-term, do you feel like this could be solved by our APIs allowing a zero value instead?

@jtaylor-cs
Copy link
Contributor Author

Hey @jtaylor-cs -

  • Just to double-check, will these changes have any impact on the current users who use retention rules via Terraform? You should be able to perform a quick check by; running the code once on the latest version available (0.0.66) and then switching to your build to see if there are any differences which may come up as a result - if this will start spitting out differences for existing users, we should document this somewhere.
  • Question (optional); is there any API changes which we can suggest to our team that would make this experience better? Whilst this will fix stuff in short-term, do you feel like this could be solved by our APIs allowing a zero value instead?

@BartoszBlizniak, I followed your instructions to run the latest version and switch to the local build to see what differences occur. I edited the PR Overview to highlight the expectations. Please let me know if I need to expand the statement or place it in another documentation section.

I consider API changes, and I don't think there are any at this time. The retention rule code recognizes a zero value as something that is skipped in the logic, so these updates actually reinforce that functionality, ensuring the user's desired value is set and persisted, thereby leading to the expected retention rule evaluation.

Copy link
Member

@BartoszBlizniak BartoszBlizniak left a comment

Choose a reason for hiding this comment

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

lgtm

@jtaylor-cs jtaylor-cs merged commit 8f51679 into master Dec 11, 2025
7 of 11 checks passed
@jtaylor-cs jtaylor-cs deleted the CENG-590_Retention_Attributes_Recognize_Zero_Values branch December 11, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants