Skip to content

Comments

Serialize properties in impression#754

Merged
gthea merged 4 commits intoSDKS-9648_baselinefrom
SDKS-9650
Apr 25, 2025
Merged

Serialize properties in impression#754
gthea merged 4 commits intoSDKS-9648_baselinefrom
SDKS-9650

Conversation

@gthea
Copy link
Contributor

@gthea gthea commented Apr 22, 2025

Android SDK

What did you accomplish?

  • Implemented definitive methods in TreatmentManagerImpl.
  • Added serialization logic for properties.

@gthea gthea self-assigned this Apr 22, 2025
@gthea gthea marked this pull request as ready for review April 22, 2025 20:47
@gthea gthea requested a review from a team as a code owner April 22, 2025 20:47
@gthea gthea force-pushed the SDKS-9648_baseline branch from 8b367f8 to 6e5ad13 Compare April 24, 2025 13:14
Copy link
Contributor

@sanzmauro sanzmauro 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 some comments

public void enableTracking(boolean enable) {
isTrackingEnabled.set(enable);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

i love this change

Comment on lines +128 to +131
long start = System.currentTimeMillis();
while (!mScheduledTasks.isEmpty() && (System.currentTimeMillis() - start) < 500L) {
try { Thread.sleep(50); } catch (InterruptedException e) { break; }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the app goes to background we pause the task executor and sometimes tasks could end up not completing. This adds a small grace period to give them a chance to finish.

public class SplitTaskExecutorImpl extends SplitBaseTaskExecutor {

private static final int MIN_THREAD_POOL_SIZE_WHEN_IDLE = 2;
private static final int MIN_THREAD_POOL_SIZE_WHEN_IDLE = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was originally 6, it was reduced to 2 in the last rc but I'm restoring it.


@Override
public Result validate(Map<String, Object> properties) {
return Result.valid(properties, 0); // TODO implement
Copy link
Contributor

Choose a reason for hiding this comment

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

why is missing this implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented in #756

Comment on lines -14 to -31
String getTreatment(String split, Map<String, Object> attributes, boolean isClientDestroyed);

SplitResult getTreatmentWithConfig(String split, Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, String> getTreatments(List<String> splits, Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, SplitResult> getTreatmentsWithConfig(List<String> splits, Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, String> getTreatmentsByFlagSet(@NonNull String flagSet, @Nullable Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, String> getTreatmentsByFlagSets(@NonNull List<String> flagSets, @Nullable Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, SplitResult> getTreatmentsWithConfigByFlagSet(@NonNull String flagSet, @Nullable Map<String, Object> attributes, boolean isClientDestroyed);

Map<String, SplitResult> getTreatmentsWithConfigByFlagSets(@NonNull List<String> flagSets, @Nullable Map<String, Object> attributes, boolean isClientDestroyed);


// temporary methods to reduce changes in this iteration
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you removing all this methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added new ones. This is internal, no changes to the API.

@gthea gthea merged commit 78f28e5 into SDKS-9648_baseline Apr 25, 2025
3 of 7 checks passed
@gthea gthea deleted the SDKS-9650 branch April 25, 2025 19:18
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.

2 participants