New methods for Handler and SharedPV #172
Conversation
Monarda
force-pushed
the
new_handler_methods
branch
from
025a119 to
687a21b
Compare
Monarda
force-pushed
the
new_handler_methods
branch
from
687a21b to
58e5c65
Compare
There was a problem hiding this comment.
Thank you @Monarda , this PR is certainly smaller as I asked.
example/persist.py
Outdated
| def post(self, pv: SharedPV, value: Value, **_kws): | ||
| """Update timestamp of the PV and store its new value.""" | ||
| self._update_timestamp(value) | ||
|
|
||
| self._upsert(value) |
There was a problem hiding this comment.
Do I understand correctly that you mean to propagate every update of this SharedPV directly to an sqlite database file? Have you done any performance measurements? (on NFS...?)
fyi. the autosave module design avoids this potentially large performance hit by decoupling record processing from .sav file I/O with a timer and separate worker thread. create_monitor_set() configures a minimum time to hold off between saves to coalesce PVs with frequent changes. The logic being to note (by monitor) when one PV changes, wait for the hold-off time, then write out all PVs which have changed since the last write.
This lets an IOC gracefully ride through NFS server outages.
I know this is only an example, and setting the time stamp is reasonable enough, but the "persist" part scares me.
There was a problem hiding this comment.
I haven't done any performance testing on this. It's not been used outside of this example. It's not intended as any kind of replacement for autosave. It would need exactly the kind of work you suggest to be able to be used for such a purpose.
I've added a note to the comments at the top of the persist.py example to make this advice not to use as anything other than an example explicit.
I've also added a new example example/auditor.py which does something similar but ought to be much more obviously flawed. (And I've added the same kind of explicit note to its opening comments.) The auditor example is less useful but does cover a few more Handler methods.
src/p4p/server/raw.py
Outdated
| # Guard goes here because we can have handlers that don't inherit from | ||
| # the Handler base class | ||
| try: | ||
| self._handler.open(V, **kws) |
There was a problem hiding this comment.
At this point in open() and post(), **kws have been digested by self._wrap(). Is there still anything meaningful to be done with them?
There was a problem hiding this comment.
I've removed a lot of unnecessary kwargs now. These were leftover from a more complex previous version.
In future I'll probably open a new issue to discuss their use case, but they didn't belong in this PR.
src/p4p/server/raw.py
Outdated
|
|
||
| _SharedPV.post(self, V) | ||
|
|
||
| def close(self, destroy=False, sync=False, timeout=None): |
There was a problem hiding this comment.
Where do these extra arguments come in?
There was a problem hiding this comment.
I believe they may have been from an earlier version of p4p, but I'm not sure! I've removed sync, and timeout now. I've left destroy.
src/p4p/server/raw.py
Outdated
| except AttributeError: | ||
| pass | ||
|
|
||
| def close(self, pv): |
There was a problem hiding this comment.
Remove the unused pv argument.
| value = op.value().raw | ||
|
|
||
| self._update_timestamp(value) | ||
| self._upsert(value, op.account(), op.peer()) |
There was a problem hiding this comment.
Is there a better way to do this? Originally I was using pv.post(op.value()) which is much neater, but discards the information about op.account() and op.peer(). Using op.value().raw feels brittle, and will discard any data apart from the value.
|
Added some simple unit tests |
This is an attempt to simplify PR #155 so that it's simply about adding new methods (
post,open, andclose) to theHandlerandSharedPVclasses inp4p.server.raw.example\persist.pyshows what can be achieved with the newopenandpostmethods. The example uses an SQLite database to persist the values of channels to a (database) file. When the program is restarted these values are read back and automatically restored to their original channels with their original timestamps.persist.pyuses thepostfunctionality to update the values of the PVs, automatically updating their timestamps, and persisting the changed values. It uses theopenfunctionality to restore the values on restart. The use of the incrementing value shows a scenario in which it is more convenient to do this in the handler with the new methods (using the combination ofopenandpost) and would not be as simple with existing methods such asonFirstConnect.