-
Notifications
You must be signed in to change notification settings - Fork 49
fix: multi-provider hook context management #1282
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
Conversation
Fixing an issue with the MultiProvider where hook contexts and hints were being lost due to copies of the context data being created in the OpenFeature sdk evaluation. Since key evaluation of Maps using objects is done by reference, the lookup of the context during evaluation was failing, leading to errors. Signed-off-by: Mike Kitzman <mdkitzman@gmail.com>
beeme1mr
left a comment
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.
Could a test be added to verify the issue has been resolved?
Yeah, let me take a look at adding a test for this. |
lukas-reining
left a comment
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.
Thanks for the PR @mdkitzman!
I see one possible issue.
When accumulating the context in the OpenFeature client in the before hooks, the context was being re-assigned instead of merged. This change ensures that the context is merged correctly, preserving object reference. Signed-off-by: Mike Kitzman <mdkitzman@gmail.com>
jonathannorris
left a comment
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.
This looks like the correct fix to me, assigning the existing object and not creating a new one makes sense.
beeme1mr
left a comment
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.
Thanks!
This PR
Fixing an issue with the MultiProvider where hook contexts and hints were being lost due to copies of the context data being created in the OpenFeature sdk evaluation.
Since key evaluation of Maps using objects is done by reference, the lookup of the context during evaluation was failing, leading to errors.
Related Issues
Fixes #1268
Notes
Follow-up Tasks
How to test