-
Notifications
You must be signed in to change notification settings - Fork 1
feat: C local bucketing scaffolding #194
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
Open
JamieSinn
wants to merge
2
commits into
main
Choose a base branch
from
c-local-bucketing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| using System.Collections.Generic; | ||
| using DevCycle.SDK.Server.Common.Model; | ||
| using DevCycle.SDK.Server.Common.Model.Local; | ||
|
|
||
| namespace DevCycle.SDK.Server.Local.Api; | ||
|
|
||
| public class CLocalBucketing : ILocalBucketing | ||
| { | ||
| public string ClientUUID { get; } | ||
|
|
||
| public CLocalBucketing() | ||
| { | ||
| ClientUUID = System.Guid.NewGuid().ToString(); | ||
| } | ||
|
|
||
| public void InitEventQueue(string sdkKey, string options) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public BucketedUserConfig GenerateBucketedConfig(string sdkKey, string user) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public int EventQueueSize(string sdkKey) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void QueueEvent(string sdkKey, string user, string eventString) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void QueueAggregateEvent(string sdkKey, string eventString, string variableVariationMapStr) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public List<FlushPayload> FlushEventQueue(string sdkKey) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void OnPayloadSuccess(string sdkKey, string payloadId) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void OnPayloadFailure(string sdkKey, string payloadId, bool retryable) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void StoreConfig(string sdkKey, string config) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void SetPlatformData(string platformData) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public string GetVariable(string sdkKey, string userJSON, string key, TypeEnum variableType, bool shouldTrackEvent) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public byte[] GetVariable(byte[] serializedParams) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public string GetConfigMetadata(string sdkKey) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void SetClientCustomData(string sdkKey, string customData) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void StartFlush() | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
|
|
||
| public void EndFlush() | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ public class DevCycleLocalClientBuilder : DevCycleClientBuilder<DevCycleLocalCli | |
| DevCycleLocalClientBuilder> | ||
| { | ||
| private EnvironmentConfigManager configManager; | ||
| private LocalBucketing localBucketing; | ||
| private ILocalBucketing localBucketing; | ||
|
|
||
| protected override DevCycleLocalClientBuilder BuilderInstance => this; | ||
|
|
||
|
|
@@ -28,7 +28,7 @@ public DevCycleLocalClientBuilder SetConfigManager(EnvironmentConfigManager envi | |
| return this; | ||
| } | ||
|
|
||
| public DevCycleLocalClientBuilder SetLocalBucketing(LocalBucketing localBucketingWrapper) | ||
| public DevCycleLocalClientBuilder SetLocalBucketing(ILocalBucketing localBucketingWrapper) | ||
| { | ||
| localBucketing = localBucketingWrapper; | ||
| return this; | ||
|
|
@@ -43,7 +43,7 @@ public DevCycleLocalClientBuilder SetInitializedSubscriber( | |
|
|
||
| public override DevCycleLocalClient Build() | ||
| { | ||
| localBucketing ??= new LocalBucketing(); | ||
| localBucketing ??= new WASMLocalBucketing(); | ||
|
|
||
| options ??= new DevCycleLocalOptions(); | ||
|
|
||
|
|
@@ -71,7 +71,7 @@ public class DevCycleLocalClient : DevCycleBaseClient | |
| private readonly string sdkKey; | ||
| private readonly EnvironmentConfigManager configManager; | ||
| private readonly EventQueue eventQueue; | ||
| private readonly LocalBucketing localBucketing; | ||
| private readonly ILocalBucketing localBucketing; | ||
| private readonly ILogger logger; | ||
| private readonly Timer timer; | ||
| private bool closing; | ||
|
|
@@ -83,7 +83,7 @@ internal DevCycleLocalClient( | |
| DevCycleLocalOptions dvcLocalOptions, | ||
| ILoggerFactory loggerFactory, | ||
| EnvironmentConfigManager configManager, | ||
| LocalBucketing localBucketing, | ||
| ILocalBucketing localBucketing, | ||
| DevCycleRestClientOptions restClientOptions = null | ||
| ) | ||
| { | ||
|
|
@@ -341,25 +341,32 @@ public override Task<Variable<T>> Variable<T>(DevCycleUser user, string key, T d | |
| try | ||
| { | ||
| var paramsBuffer = paramsPb.ToByteArray(); | ||
| try | ||
| { | ||
| byte[] variableData = localBucketing.GetVariable(serializedParams: paramsBuffer); | ||
| if (variableData == null) | ||
| { | ||
| return Task.FromResult(Common.Model.Variable<T>.InitializeFromVariable(null, key, defaultValue, | ||
| DefaultReasonDetails.UserNotTargeted)); | ||
| } | ||
|
|
||
| byte[] variableData = localBucketing.GetVariableForUserProtobuf(serializedParams: paramsBuffer); | ||
|
|
||
| if (variableData == null) | ||
| SDKVariable_PB sdkVariable = SDKVariable_PB.Parser.ParseFrom(variableData); | ||
| var evalReason = new EvalReason(sdkVariable.Eval.Reason, sdkVariable.Eval.Details, | ||
| sdkVariable.Eval.TargetId); | ||
| existingVariable = GetVariable<T>(sdkVariable, defaultValue, evalReason); | ||
| return Task.FromResult(existingVariable); | ||
| } | ||
| catch (NotImplementedException) | ||
| { | ||
| return Task.FromResult(Common.Model.Variable<T>.InitializeFromVariable(null, key, defaultValue, DefaultReasonDetails.UserNotTargeted)); | ||
| // C Local bucketing | ||
| } | ||
|
|
||
| SDKVariable_PB sdkVariable = SDKVariable_PB.Parser.ParseFrom(variableData); | ||
| var evalReason = new EvalReason(sdkVariable.Eval.Reason, sdkVariable.Eval.Details, sdkVariable.Eval.TargetId); | ||
| existingVariable = GetVariable<T>(sdkVariable, defaultValue, evalReason); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| logger.LogError("Unexpected exception getting variable: {Exception}", e.Message); | ||
| return Task.FromResult(Common.Model.Variable<T>.InitializeFromVariable(null, key, defaultValue, DefaultReasonDetails.Error)); | ||
| } | ||
|
|
||
| return Task.FromResult(existingVariable); | ||
| return Task.FromResult(Common.Model.Variable<T>.InitializeFromVariable(null, key, defaultValue, DefaultReasonDetails.Error)); | ||
| } | ||
|
|
||
| public async Task<Variable<T>> VariableAsync<T>(DevCycleUser user, string key, T defaultValue) | ||
|
|
@@ -387,7 +394,7 @@ public async Task<Variable<T>> VariableAsync<T>(DevCycleUser user, string key, T | |
| var type = Common.Model.Variable<T>.DetermineType(defaultValue); | ||
| VariableType_PB variableType = TypeEnumToVariableTypeProtobuf(type); | ||
|
|
||
| VariableForUserParams_PB paramsPb = new VariableForUserParams_PB() | ||
| VariableForUserParams_PB paramsPb = new VariableForUserParams_PB | ||
| { | ||
| SdkKey = sdkKey, | ||
| User = userPb, | ||
|
|
@@ -407,19 +414,19 @@ public async Task<Variable<T>> VariableAsync<T>(DevCycleUser user, string key, T | |
|
|
||
| try | ||
| { | ||
| System.Exception beforeError = null; | ||
| Exception beforeError = null; | ||
| try | ||
| { | ||
| hookContext = await evalHooksRunner.RunBeforeAsync(hooks, hookContext); | ||
| } | ||
| catch (System.Exception e) | ||
| catch (Exception e) | ||
| { | ||
| beforeError = e; | ||
| } | ||
|
Comment on lines
+422
to
425
|
||
|
|
||
| var paramsBuffer = paramsPb.ToByteArray(); | ||
|
|
||
| byte[] variableData = localBucketing.GetVariableForUserProtobuf(serializedParams: paramsBuffer); | ||
| byte[] variableData = localBucketing.GetVariable(serializedParams: paramsBuffer); | ||
|
|
||
| if (variableData == null) | ||
| { | ||
|
|
@@ -431,7 +438,7 @@ public async Task<Variable<T>> VariableAsync<T>(DevCycleUser user, string key, T | |
|
|
||
| SDKVariable_PB sdkVariable = SDKVariable_PB.Parser.ParseFrom(variableData); | ||
| var evalReason = new EvalReason(sdkVariable.Eval.Reason, sdkVariable.Eval.Details, sdkVariable.Eval.TargetId); | ||
| existingVariable = GetVariable<T>(sdkVariable, defaultValue, evalReason); | ||
| existingVariable = GetVariable(sdkVariable, defaultValue, evalReason); | ||
| string featureValue = sdkVariable.Feature?.IsNull == false ? sdkVariable.Feature.Value : null; | ||
| variableMetadata.FeatureId = featureValue; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| using System.Collections.Generic; | ||
| using DevCycle.SDK.Server.Common.Model; | ||
| using DevCycle.SDK.Server.Common.Model.Local; | ||
|
|
||
| namespace DevCycle.SDK.Server.Local.Api; | ||
|
|
||
| public interface ILocalBucketing | ||
| { | ||
| public string ClientUUID { get; } | ||
| public void InitEventQueue(string sdkKey, string options); | ||
| public BucketedUserConfig GenerateBucketedConfig(string sdkKey, string user); | ||
| public int EventQueueSize(string sdkKey); | ||
| public void QueueEvent(string sdkKey, string user, string eventString); | ||
| public void QueueAggregateEvent(string sdkKey, string eventString, string variableVariationMapStr); | ||
| public List<FlushPayload> FlushEventQueue(string sdkKey); | ||
| public void OnPayloadSuccess(string sdkKey, string payloadId); | ||
| public void OnPayloadFailure(string sdkKey, string payloadId, bool retryable); | ||
|
|
||
| public void StoreConfig(string sdkKey, string config); | ||
| public void SetPlatformData(string platformData); | ||
| public string GetVariable(string sdkKey, string userJSON, string key, TypeEnum variableType, bool shouldTrackEvent); | ||
| public byte[] GetVariable(byte[] serializedParams); | ||
| public string GetConfigMetadata(string sdkKey); | ||
| public void SetClientCustomData(string sdkKey, string customData); | ||
| public void StartFlush(); | ||
| public void EndFlush(); | ||
|
|
||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The catch block for
NotImplementedExceptionis empty with only a comment indicating 'C Local bucketing'. After catching this exception, the code falls through to line 369 which returns a default variable withDefaultReasonDetails.Error. This flow is unclear. If this is intentional fallback behavior for C Local bucketing, consider either: (1) adding an explicit comment explaining why the error default is returned, or (2) implementing the C Local bucketing path here if it differs from the error case.