-
Notifications
You must be signed in to change notification settings - Fork 283
switch ZFS volume backend to pyzfs / libzfs_core #1781
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
b3770ed
d6cbfaf
e5531f6
9a8d17c
5f66c2d
fa0a0ff
9a80397
a44576f
bcd8bf1
8d8a6d8
1349bfd
35ce7c2
f4eb274
186eff4
8308e8a
3d8f9cb
dcc914f
4d2bbdb
e387cf5
47dfab0
3123937
7cfdc3a
80c1a02
05fb5d5
ead482e
3801ea8
8ba3807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,15 +72,40 @@ def connectionLost(self, reason): | |
|
|
||
|
|
||
| class _AsyncLZC(object): | ||
| """ | ||
| A proxy class for the asynchronous execution using a given reactor and its | ||
| thread pool. | ||
|
|
||
| Primarily this class dispatches its method calls to the functions in | ||
| :mod:`libzfs_core`. But it can also be used for the asynchronous execution | ||
| of an arbitrary function. | ||
| """ | ||
|
|
||
| def __init__(self, reactor): | ||
| """ | ||
| :param reactor: the reactor that is to be used for the asynchronous | ||
| execution. | ||
| """ | ||
| self._reactor = reactor | ||
| self._cache = {} | ||
|
|
||
| def callDeferred(self, func, *args, **kwargs): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just move this into the nested _async_wrapper function below?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's actually not private and is used in several places. The missing documentation would help, of course. |
||
| """ | ||
| This is a thin wrapper around :func:`deferToThreadPool`. | ||
|
|
||
| Its primary advantage is that the reactor is already associated with | ||
| an instance of :class:`_AsyncLZC` and :meth:`getThreadPool` is called | ||
| to get the reactor's thread pool. | ||
| """ | ||
| return deferToThreadPool(self._reactor, self._reactor.getThreadPool(), | ||
| func, *args, **kwargs) | ||
|
|
||
| def __getattr__(self, name): | ||
| """ | ||
| Pretend that this class provides the same methods as the functions | ||
| in :mod:`libzfs_core`. The proxy methods execute the functions | ||
| in the asynchronous mode using the reactor and its thread pool. | ||
| """ | ||
| try: | ||
| return self._cache[name] | ||
| except KeyError: | ||
|
|
@@ -95,6 +120,16 @@ def _async_wrapper(*args, **kwargs): | |
|
|
||
|
|
||
| class _FakeAsyncLZC(object): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe move the Fake to a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _FakeAsyncLZC has been completely removed as it has become disused now, even by the tests. |
||
| """ | ||
| A proxy class that emulates the asynchronous execution. | ||
|
|
||
| This class simulates behavior of :class:`_AsyncLZC`, but all the calls | ||
| are actually synchronous and returned :class:`Deferred` objects already | ||
| have results. | ||
|
|
||
| This is useful for testing when a reactor used does not have a thread pool. | ||
| """ | ||
|
|
||
| def __init__(self): | ||
| self._cache = {} | ||
|
|
||
|
|
@@ -119,6 +154,15 @@ def _async_wrapper(*args, **kwargs): | |
|
|
||
|
|
||
| def _async_lzc(reactor): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this looks like a function that'd only be used in tests...returning FakeAsyncLZC for synchronous unit tests and AsyncLZC for async functional tests.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The missing documentation would help here as well :-( |
||
| """ | ||
| Return an instance of either :class:`_AsyncLZC` or :class:`_FakeAsyncLZC` | ||
| for the given reactor depending on its capabilities. | ||
|
|
||
| :param reactor: the reactor. | ||
|
|
||
| The instance gets associated with the reactor and the same instance will | ||
| be returned for subsequent calls with the same ``reactor`` argument. | ||
| """ | ||
| try: | ||
| return _reactor_to_alzc[reactor] | ||
| except KeyError: | ||
|
|
||
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 know it's early days, but a doc string here would help me understand the design of this wrapper.
Instead of wrapping every libzfs_core function with
_async_wrapperconsider creating anILibZFSCoreZope Interface and usingauto_threadedfrom https://github.com/ClusterHQ/flocker/blob/master/flocker/common/_thread.pyOr perhaps you'll have to create a slightly higher level abstraction in order to use that. eg
IZFSClientwhich is wrapped to giveIZFSClientAsync. That's how @exarkun organised theIBlockDeviceAPI.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 sounds a little bit too advanced for my skills.
I can try to do that or it can be deferred to a more skilled developer.
In either case I will work on this item and see what I can do. At the very least I'll add documentation.
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.
At least, I've added the docstrings now.