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

Extend hanami_context_logging to handle non-hash ContextProvider#context #5

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

sswander
Copy link
Collaborator

But this at least it has to be convertible to hash via #to_h

This is so that we can be compatible with Oculus 0.2.1, which changed the Context from actual ruby hash to be an object (with more capabilities) -- see https://github.com/Kaligo/oculus/pull/3/files#diff-3eb728766f4a509971570cab11b09e56c1ae9c480dad013ecd314fa80ba321d6L10

not a hash, but responds to #to_h

This is so that we can be compatible with Oculus 0.2.1, which changes
the Context from a hash to be an object (with more capabilities)
@sswander sswander requested a review from tsungtingdu October 25, 2024 08:55
Comment on lines +20 to +23
provider_context = if @context_provider.context.is_a?(Hash)
@context_provider.context
else
@context_provider.context.to_h
Copy link

Choose a reason for hiding this comment

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

Maybe we can always call to_h since Hash#to_h returns itself

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's what @tsungtingdu and I also mentioned, but I thought this is more intentional :keke:

@tsungtingdu tsungtingdu added the code approved Code approved, good to test in STG label Oct 28, 2024
@tsungtingdu
Copy link

@sswander it works

Screenshot 2025-01-22 at 10 45 57 (2)

fyi, I still need to add .to_h when using context

transaction.set_tags(Oculus::Core.context.to_h)

@sswander
Copy link
Collaborator Author

fyi, I still need to add .to_h when using context

That is true... this is on Oculus side then. Thanks Tim 🙏 Meanwhile I'll bump the version and merge this PR

@sswander sswander merged commit 6e729d2 into master Jan 22, 2025
@sswander
Copy link
Collaborator Author

@tsungtingdu you should be able to use this tag now like this!

gem 'hanami-context-logging', ascenda_private: 'Kaligo/hanami-context-logging', tag: 'v0.1.3'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code approved Code approved, good to test in STG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants