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.

Sign up for GitHub

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

Jump to bottom

[MISC] update wrapper methods context #187

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[MISC] update wrapper methods context #187

wants to merge 1 commit into from

Conversation

gboutry
Copy link

@gboutry gboutry commented Jul 31, 2024

Update the wrapper methods context to the wrapped method.

Fix #186

This will make it easier to follow traced method in tracing tools (or even logs, if print method name in them)

juditnovak
juditnovak approved these changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@juditnovak juditnovak left a comment

Choose a reason for hiding this comment

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

Thank you very much for the fix/enhancement :-)

welpaolo
welpaolo reviewed Aug 14, 2024
View reviewed changes
Copy link
Contributor

@welpaolo welpaolo left a comment

Choose a reason for hiding this comment

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

Looks good to me! Please update LIBPATCH version to avoid publishing issues

lib/charms/data_platform_libs/v0/data_interfaces.py Outdated Show resolved Hide resolved
@gboutry
Copy link
Author

gboutry commented Aug 14, 2024

Force push is rebase on main + incrementing libpatch

@juditnovak juditnovak mentioned this pull request Aug 29, 2024
Draft
@juditnovak
Copy link
Contributor

juditnovak commented Aug 29, 2024 •
edited
Loading

@gboutry A one-character fix is required so that the tests could pass. I aplogize it's my fault.
(We need the pipeline to be green so we could merge...)

You can find the fix on my replica of your PR #192

@gboutry
[MISC] update wrapper methods context
1accd1d 
  * Update the wrapper methods context to the wrapped method.
  * Increase libpatch

Signed-off-by: Guillaume Boutry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@welpaolo welpaolo welpaolo left review comments

@juditnovak juditnovak juditnovak approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wrapper methods don't update their context, making harder to trace / follow in logs
3 participants
@gboutry @juditnovak @welpaolo