Dear Orthanc team,
I implemented the StorageCommitmentScpCallback for Python and would like to have some feedback.
View the patch diff on GitHub
The patch is uploaded to GitHub for ease of review: https://github.com/j3soon/orthanc-python-storage-commitment/pull/1/files
Since the patch is not trivial (as you mentioned), I would like to ask for your suggestions here before submitting it.
If no further modifications are required, I will submit a patch into the Orthanc internal code following the contribution guidelines (i.e., sign and email a CLA to Osimis & submit a Mercurial patch, as mentioned in https://book.orthanc-server.com/developers/repositories.html#submitting-code )
Description
The original C++ callback is defined as: (ref: https://sdk.orthanc-server.com/group__DicomCallbacks.html#ga86bf2a20f48e22c74bc18d589e51bb02)
OrthancPluginErrorCode
OrthancPluginRegisterStorageCommitmentScpCallback(
OrthancPluginContext* context,
OrthancPluginStorageCommitmentFactory factory,
OrthancPluginStorageCommitmentDestructor destructor,
OrthancPluginStorageCommitmentLookup lookup
)
- The
factory callback receives the DICOM Storage Commitment information, and returns a new object handler for later use.
- The
lookup callback is called once for each instance in the DICOM Storage Commitment, and returns an error code for the current instance. It can access the handler constructed in factory.
- The
destructor callback deconstructs the handler created in factory.
Python utilizes reference counting, so we don’t need to explicitly provide a destructor callback. If we want to use a custom destructor, simply use a custom type and define __del__. (In fact, Orthanc does not call the destructor immediately after responding to the DICOM Storage Commitment request. The destructor is called immediately before Orthanc stops/exits. I’m not sure if this is a bug in Orthanc, but this issue does not affect the implementation of the Python callback.)
Therefore, the Python RegisterStorageCommitmentScpCallback should only contain 2 callbacks: factory and lookup.
Below is a Python plugin equivalent to the C++ sample plugin: (https://hg.orthanc-server.com/orthanc/file/tip/OrthancServer/Plugins/Samples/StorageCommitmentScp/Plugin.cpp )
import orthanc
class CallbackData(object):
def __init__(self):
print("Constructor called.")
self.count = 0
def __del__(self):
print("Destructor called.")
def count_post_increment(self):
self.count += 1
return self.count - 1
def StorageCommitmentScpCallback(jobId, transactionUid, sopClassUids, sopInstanceUids, remoteAet, calledAet):
print(jobId, transactionUid, remoteAet, calledAet)
assert len(sopClassUids) == len(sopInstanceUids)
for i in range(len(sopClassUids)):
print("++", sopClassUids[i], sopInstanceUids[i])
data = CallbackData() # any Python native type or self-defined class
return data
def StorageCommitmentLookup(sopClassUid, sopInstanceUid, data):
print("??", sopClassUid, sopInstanceUid)
if data.count_post_increment() % 2 == 0:
return orthanc.StorageCommitmentFailureReason.SUCCESS
else:
return orthanc.StorageCommitmentFailureReason.NO_SUCH_OBJECT_INSTANCE
orthanc.RegisterStorageCommitmentScpCallback(StorageCommitmentScpCallback, StorageCommitmentLookup)
Details of File Changes
The following are the modification details of this patch:
-
.gitignore & .hgignore:
For visualization purpose on GitHub, they won’t be included in the final patch.
-
CMakeLists.txt:
Requires compiling StorageCommitmentScpCallback.cpp.
-
NEWS:
The Change Log
-
Sources/Plugin.cpp:
Includes StorageCommitmentScpCallback.h and defines RegisterStorageCommitmentScpCallback for Python. The (TODO) in the comment: New in release (TODO) will be changed to the next release number.
-
Sources/ICallbackRegistration.h & Sources/ICallbackRegistration.cpp:
Add Apply2 for adding Python callback that receives 2 arguments.
-
Sources/StorageCommitmentScpCallback.h & Sources/StorageCommitmentScpCallback.cpp:
Code for porting OrthancPluginRegisterStorageCommitmentScpCallback in C++ to RegisterStorageCommitmentScpCallback in Python.
-
Samples/*:
3 python plugin samples for testing the callback, which are equivalent to the C++ sample plugin: (https://hg.orthanc-server.com/orthanc/file/tip/OrthancServer/Plugins/Samples/StorageCommitmentScp/Plugin.cpp ) . They won’t be included in the final patch for orthanc-python, but I think they may be included in an additional patch for orthanc-tests in https://hg.orthanc-server.com/orthanc-tests/file/default .
