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

Allow TransformAction to clone options #64

Conversation

DeepRed
Copy link
Collaborator

@DeepRed DeepRed commented Dec 12, 2024

When the client is used to query different graph
sources the transform action passed to
CreateFromConfig can be used to manipulate the
keys sent. However the changes are applied to the
global options and doesnt reset after the client
has finished its query.

By introducing a behaviour it is possible to
dictatate if the developer wants to be able to
apply the transformation on a clone of the
options.

The default behaviour is that the current behavior where the options are global.

@CLAassistant
Copy link

CLAassistant commented Dec 12, 2024

CLA assistant check
All committers have signed the CLA.

@@ -42,6 +42,14 @@ public static GraphQueryBuilder CreateFromConfig(Action<OptiGraphOptions> transf
var httpClientFactory = ServiceLocator.Current.GetService(typeof(IHttpClientFactory)) as IHttpClientFactory;
if (transformAction != null)
{
if (options.TransformActionBehaviour == TransformActionBehaviour.Clone)
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I added the TransformActionBehaviour is to avoid changing the behaviour which could potentially be a breaking change. By keeping the Default behaviour a customer can upgrade an existing solution without configuring this option.
But I recognize that this might be unnecessary depending on which use cases that exists. Please keep in mind and consider during review.

@ManhOptimizely ManhOptimizely force-pushed the feature/CMS-38272-clone-options-for-multi-graph-sources branch from 804101c to af7e021 Compare December 16, 2024 10:40
Copy link

Great job @ManhOptimizely! Your CI passed, and the PR has been automatically labelled.

Once ready, we will merge this PR to trigger the next part of the automation 🚀

@ManhOptimizely ManhOptimizely changed the base branch from main to develop December 16, 2024 10:49
When the client is used to query different graph
sources the transform action passed to
CreateFromConfig can be used to manipulate the
keys sent. However the changes are applied to the
global options and doesnt reset after the client
has finished its query.

By introducing a behaviour it is possible to
dictatate if the developer wants to be able to
apply the transformation on a clone of the
options.

The default behaviour is that the current behavior
where the options are global.
@ManhOptimizely ManhOptimizely force-pushed the feature/CMS-38272-clone-options-for-multi-graph-sources branch from af7e021 to 5f357f6 Compare December 17, 2024 01:07
Copy link

Great job @ManhOptimizely! Your CI passed, and the PR has been automatically labelled.

Once ready, we will merge this PR to trigger the next part of the automation 🚀

@ManhOptimizely ManhOptimizely merged commit 5f357f6 into develop Dec 17, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants