-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix PyAction OpmLog #4440
Fix PyAction OpmLog #4440
Conversation
jenkins build this please |
4b55ea1
to
0f0539c
Compare
jenkins build this please |
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.
I assume the Python parts here do what you need, so I've mostly looked at the C++ parts. It mostly looks good to me, with a couple of points that could use some clarification.
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.
I've now added a test for this (07fb927) and made the test pass.
@lisajulia Are you referring to #4441 ? Seems like the last jenkins build there failed?
0f0539c
to
b482f65
Compare
b482f65
to
dbc2b4e
Compare
Yes exactly :) |
jenkins build this please |
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 a lot for the updates. The C++ parts here look okay now. Let's await Python parts review before merging.
This PR fixes logging messages with the OpmLog from embedded Python code.
This used to work and then stopped working at some point, I've now added a test for this (07fb927) and made the test pass.
The issue was that the internal logger of the static class OpmLog was not set when OpmLog was called from embedded python code.
I have added a function setLogger to the OpmLog such that the Logger can be set using a python function on the python object OpmLog.
I'm happy to discuss other approaches to this!