Skip to content

Conversation

@rathovarun1032
Copy link
Contributor

@rathovarun1032 rathovarun1032 commented Aug 14, 2025

This resolves two bugs identified during the recent bug bash related to server template evaluation and parameter handling.

Corrected Custom Signal Evaluation with Cached Templates
A bug was preventing custom signals from being evaluated correctly when the server template was initialized from a cache. The root cause was an issue with template serialization/deserialization in ServerVersion and Condtion. The key percentOperator was being used instead of the correct percentConditionOperator. This has been fixed, ensuring that cached templates are parsed correctly and evaluations work as intended.

Fixed Malformed Logging Warning for getDouble()
When getDouble() was called on a parameter with a string value, it correctly returned the default static value but produced a malformed warning in the logs (e.g., "Unable to convert %s to double type."). The string formatting in the logger has been corrected to properly display the value that failed conversion.

Athira M and others added 11 commits August 7, 2025 22:48
…ersion failure)

Using getDouble on a string parameter value, returns the appropriate default static value but logs a warning which looks incorrect ("%s" in the warning message?).
In the server template builder flow using cached template, evaluation using custom signals is not working as intended.
Copy link

@ashish-kothari ashish-kothari left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM! Added one comment. Thanks!

@rathovarun1032 rathovarun1032 merged commit 3f75c4e into ssrc Aug 28, 2025
6 checks passed
rathovarun1032 added a commit that referenced this pull request Sep 23, 2025
* Implementation for Fetching and Caching Server Side Remote Config (#1107)009

* Implementation for Fetching and Caching Server Side Remote Config

* implementation of fetch , cache and load of template

---------

Co-authored-by: Varun Rathore <varunrathore@google.com>

* Implement custom signal targeting for server side RC (#1108)

Co-authored-by: Athira M <athiramanu@google.com>

* Implement percent evaluation for server side RC (#1114)

* [feat] Implement percent evaluation for server side RC

* Ssrc bugbash fix (#1117)

* Handle empty context

* fix issue related to update time

* fix string equality

* fix textcase

* Fix lint errors

* Add unit tests

* fix for [438426692](getDouble() logs a malformed warning on type conversion failure)

Using getDouble on a string parameter value, returns the appropriate default static value but logs a warning which looks incorrect ("%s" in the warning message?).

* Update ServerTemplateResponse.java to fix b/438607881

In the server template builder flow using cached template, evaluation using custom signals is not working as intended.

* Update getServerRemoteConfig.json to fix b/438607881

* Update getServerTemplateData.json to fix b/438607881

* fix for bugs

* Resolve comment related to revert of ServerVersion Class

* remove serverVersion

* Resolve comments related to Evaluator

* fix indentation

* fix indentation

* fix indentations

* fix multi line indent

* fix multi line indents

* Update ConditionEvaluator.java

* Update ConditionEvaluator.java

---------

Co-authored-by: Athira M <athiramanu@google.com>
Co-authored-by: Varun Rathore <varunrathore@google.com>

* Create ParameterValueTest.java

* Fix typo errors

* Change return type and cache regex

* Addressed comment to make cache atomic

* Trigger CI

---------

Co-authored-by: Varun Rathore <varunrathore@google.com>
Co-authored-by: Athira M <athiramanu9400@gmail.com>
Co-authored-by: Athira M <athiramanu@google.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