-
Notifications
You must be signed in to change notification settings - Fork 2
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
Disk/memory modes of PersHelper #25
Conversation
ad7c34f
to
f4f6b56
Compare
…rsistence, add unit tests.
f4f6b56
to
c7b8af6
Compare
src/scorep_jupyter/kernel.py
Outdated
self.pershelper = PersHelper('cloudpickle') | ||
|
||
self.cell_output(f'Serializer backend switched to {serializer}, persistence was reset.') | ||
if serializer in ['dill', 'cloudpickle']: |
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.
can we have something non-static here, e.g. a config file so that we could support user-defined serializer as well? In the future, we don't want to change the code to plugin an alternative serializer.
def serializer_settings(self, serializer, mode): | ||
error_message = "" | ||
|
||
if serializer in ['dill', 'cloudpickle']: |
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.
please also catch the following cases:
- no serializer set -> use default one
- no communication set -> use default one
for both cases, please print the whole configuration (serializer and mode that is used)
if os.path.exists(scorep_script_name): | ||
os.remove(scorep_script_name) | ||
|
||
def serializer_settings(self, serializer, mode): |
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.
following the comment from the other conversation, please try to make it as configurable as possible, i.e. in the future we might want to add custom serializers here
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.
please also update the README about the new magic command and the environment variable for the persistence directory.
3a3ae4a
to
1bb31cb
Compare
1bb31cb
to
92d90a8
Compare
Extend
PersHelper
class to support "disk" and "memory" ways of communicating the persistence between Jupyter and subprocess - using either files or pipes respectively.PersHelper
can now be configured with the following magic: