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
Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320
Conversation
@@ -68,6 +68,30 @@ | |||
:meth:`~pywbem.WBEMConnection.GetQualifier` Retrieve a qualifier declaration | |||
:meth:`~pywbem.WBEMConnection.SetQualifier` Create or modify a qualifier declaration | |||
:meth:`~pywbem.WBEMConnection.DeleteQualifier` Delete a qualifier declaration | |||
------------------------------------------------------- --------------------------------------------------------- |
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.
Suggest to move the entire group before class ops.
COMMENT KS: Agreed. Will continue in Col Spr
Update with new proposal for the interface. This does not change the internal issues between tupleparse and the response processing but it passes the limited tests in run_cimoperations.py. Mock not fixed at this point so will fail tests. |
I like the change to return a tuple, including the idea to hide the namespace in an inner tuple. We could use namedtuple for the outer tuple. |
A tuple containing: | ||
* A list of :class:`~pywbem.CIMInstance` objects | ||
that are representations of the enumerated instances. | ||
end_of-sequence (:class:`py:bool`). If `True` this enumeration |
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.
Arg name: Use underscore instead of hyphen.
Missing asterisk before arg name.
On 05/23/2016 08:43 AM, Andreas Maier wrote:
My only concern is to be sure the namedtuple applies all the way back to
Karl Schopmeyer |
I just checked, and namedtuple was added in py 2.6: |
the query parameter in a namespace | ||
:meth:`~pywbem.WBEMConnection.PullInstances` Continue enumeration of an enumeration session opened | ||
with OpenExecQuery | ||
:meth:`~pywbem.WBEMConnection.CloseEnumeration` Close an enumeration session in process. |
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 suggest adding a note at the bottom pointing out that EnumerationCount is not supported.
6da1db7
to
e38fc65
Compare
Karl, please rebase this PR on branch fix_331_builddoc-failure-on-py26 (or review that branch quickly so I can merge it, and then rebase this one on master). |
1accd9b
to
7683403
Compare
Agreed with rebase requirement. Having a real issue here that I need another eye on in that rebase tries to discard my code so I have done something nasty in some past push/rebase/etc. that is causing real confusion for git. This is what happened the other day and it happened again last night. Just drops most of my changes. |
See my mail on cleaning it up. |
7683403
to
152f073
Compare
Sorry. merging another branch caused another need for rebase. |
152f073
to
bd8e08a
Compare
+1 |
We discussed two issues here based on the latest push;
|
TODO Andy: Final review of the code. |
On Karl's items:
DISCUSSION, KS: This one is more interesting. Original code. The EnumerateInstances and EnumerateInstancePaths does: for namespace in namespace NOTE: I am assuming that the setattr code I have commented would be more efficient this this can be done thousands of times on a single response. However, the associators/references and their name equivalents don't fix the namespace That is correct, I believe' because we do not know the namespace of an instance reference or pull. With the pull sequence we have common code, the same PullInstancesWithPath handles both OpenEnumerateInstances and OpenAssociatorInstances/OpenRefereneInstances. We not only have to handle the opens, but the pulls. Thus, we must now do ithe namespace test either:
OR: We have to carry extra info in the context between open and pulls to determine whether to fix the namespace entites. Or we just wait and then just log operation responses that return paths that have an error. |
Additional comments, focusing on the code:
No problem. I really wanted to do a function:
corresponding one for instances. and in each function simply: but that hides the named tuple behind another layer and makes it a bit harder for the someone |
We agreed in the call to:
COMMENT: DONE
COMMENT: you do need to return as *self...) to force the unpack of the tuple. DONE |
EnumerationCount which will NOT be implemented because it has been deprecated. These operations are implemented consistent with the DMTF specifications and with the APSs approximately the same as DSP0200. The major difference is that we carry the namespace from operation internally within the context that must be passed between operations since it is a constant once the open has been executed. All of the pull operations defined in DSP0200 are implemented except for EnumerationContext. We will not implement that request because it is being deprecated by DMTF as of no value. The api documentation for all of these operations is up-to-date and has passed multiple reviews. Implements the result of all of the pull operations as a named tuple with three parts, instances/paths, end_of_sequence, enumeration_context. The OpenQueryInstances has 4 parts to account for the ResultClass parameter. Adds INSTANCEPATH as IRETURNVALUE child in tupleparse.py as they are used by the pull operations xml. The live test (run_cim_operations.py) has been extended to run tests on all of the pull operations. Today this tests largely the main paths and the core parameters on all of the operations. We are missing tests for a lot of edge cases and for the extra paramters on associators, references, etc. These tests have been successfully run against openpegasus with run_cimoperations.py. API documentation is complete. Tests exist for all pull functions as part of run_cimoperations.py and they have been tested against OpenPegasus. This pr does NOT include changes to wbemcli for these new operations. test_client.py was extended to process the results of the pull operations since there is a key difference between the operation and the others in that these return a tuple of results (objects, paramvalues) rather than just objecs. We added a new response type that processes the tuple results. There is one mock testcase for openEnumerateInstances. Note added to changes.rst about this pr and the new methods. Adds a single example of a pull enumerate instances session. This example uses the pull to enumerate instances, display information on each operation and display the resulting list of instances Last version of pr fixes pull operations per comments including: 1. Modified pull result to globals for namedtuples. 2. Remove commented code. 3. Simplify pull result code so it is now a single statement 4. Modified run_cimoperations.py to clean up tests and fix some errors in tests. There is still one test that takes a long time because it runs against a whole namespace.
bd8e08a
to
c35d540
Compare
+1 Changes made from comments plus some other issues I found. |
This commit includes OpenEnumerateInstancesWithPath,
PullInstancesWithPath, CloseEnumeration, OpenEnumerateInstancePaths,
and PullInstancePaths. The documentation is defined for all but
OpenEnumerateInstancePaths and PullInstancePaths.
The mock test is broken but there is a first set of tests in
run_cimoperations.py that executes a number of variations of
OpenEnumerateInstancePaths with pull and close against OpenPegasus.
NOT READY FOR MERGE.
Here is first cut of part ready for review and comment. In particular, this currently fails mock and only about hallf of operaitons there. Since the documentaiton for each operation is enormous would like to get the first one clean (OpenEnumerateInstances) before doing the other opens.
Also some code is hacky and we should discuss the enum_context thing class.
Anyway, it actually works now.