Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320

KSchopmeyer · 2016-05-21T03:24:42Z

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.

andy-maier · 2016-05-21T10:51:48Z

pywbem/cim_operations.py

@@ -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
+------------------------------------------------------- ---------------------------------------------------------


Suggest to move the entire group before class ops.

COMMENT KS: Agreed. Will continue in Col Spr

KSchopmeyer · 2016-05-23T02:59:18Z

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.

andy-maier · 2016-05-23T14:43:39Z

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.

andy-maier · 2016-05-23T14:47:04Z

pywbem/cim_operations.py

+            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


Arg name: Use underscore instead of hyphen.
Missing asterisk before arg name.

KSchopmeyer · 2016-05-23T22:41:34Z

On 05/23/2016 08:43 AM, Andreas Maier wrote:

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.

My only concern is to be sure the namedtuple applies all the way back to
2.6. Will check

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#320 (comment)

Karl Schopmeyer
email: k.schopmeyer@swbell.net, k.schopmeyer@opengroup.org
tel: 972-814-5581

andy-maier · 2016-05-24T00:25:37Z

I just checked, and namedtuple was added in py 2.6:
https://docs.python.org/2/library/collections.html#collections.namedtuple

andy-maier · 2016-05-24T05:23:46Z

pywbem/cim_operations.py

+                                                        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.


I suggest adding a note at the bottom pointing out that EnumerationCount is not supported.

andy-maier · 2016-05-30T14:16:19Z

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).

KSchopmeyer · 2016-06-06T12:39:58Z

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.

andy-maier · 2016-06-06T22:17:20Z

See my mail on cleaning it up.

andy-maier · 2016-06-07T04:46:01Z

Sorry. merging another branch caused another need for rebase.
If you want, we can do that in our call today, to see what goes wrong.

KSchopmeyer · 2016-06-07T13:53:16Z

+1

KSchopmeyer · 2016-06-07T13:58:24Z

We discussed two issues here based on the latest push;

We ignore unknown iparamvalue names in the response. Should we error, iginore, log?
We bypassed the test of namespace on responses. Should we leave it as is, check and log issue, or fix the responses? Remember that while theoriginal enum did add them, this is a high cost function for thousands of responses and it is clearly a server error.

andy-maier · 2016-06-07T13:58:52Z

TODO Andy: Final review of the code.

andy-maier · 2016-06-07T14:23:58Z

On Karl's items:

handling of unknown iparamvalues in reponse: ignore and log. Added the logging part to issue Support logging to the system log #294. That leaves the "ignore" part to be implemented.

COMMENT: KS That is what the code does today. Extra IPARAMVALUES just slide through with no notice. When we do logging we can log these issues.

i'd prefer if we could set the namespace in the path if has not been set by the server, and also log that fact. Added the logging part to issue Support logging to the system log #294. That leaves the "setting it, if not set" part to be implemented.

DISCUSSION, KS: This one is more interesting. Original code. The EnumerateInstances and EnumerateInstancePaths does:

for namespace in namespace
set the namespace in path.

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:

None of the open/pull operations
or
all of the open/pull operations that return paths
Whereas in the original code only the EnumerateInstances and names completed namespace.

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.

andy-maier · 2016-06-07T21:10:08Z

Additional comments, focusing on the code:

type name of namedtuple used for results: e.g. line 1587 of cim_operations.py:
```
rslttup = namedtuple("result", ["paths", "eos", "context"])
```
rslttup is a type and "result" is its name (i.e. rslttup.__name__ = "result"). From a type T, one expects T.__name__ == "T".

Also, the type rslttup is a bit too abbreviated, in my mind.

How about:
```
pull_result_tuple = namedtuple("pull_result_tuple", ["paths", "eos", "context"])
```
Appears a number of times.

No problem. I really wanted to do a function:

def _getPathResult(self, result):
""" parse the returned result and generate a named tuple for the response"""
``
paths, end_of_seq, enum_ctxt = self._get_rslt_params(result, namespace)
` pull_result_tuple = namedtuple("result", ["paths", "eos", "context"])`
` return pull_result_tuple(paths, end_of_seq, enum_ctxt)`

corresponding one for instances.

and in each function simply:
return _getPathResult(result)

but that hides the named tuple behind another layer and makes it a bit harder for the someone
reviewing the code to see that a named tuple is being returned so left it as is for now.

andy-maier · 2016-06-08T16:09:59Z

We agreed in the call to:

Move the two different result tuple types to become global variables in the cim_operations module (they stay within that module, i.e. are not added to __all__ and do not get otherwise added to the pywbem namespace. The types will have a name that is their type name (i.e. T.__name__ == "T"). For example:
```
pull_path_tuple = namedtuple("pull_path_tuple", ["paths", "eos", "context"])
```

COMMENT: DONE

In each function that returns these named tuples, create the tuple explicitly instead of hiding it in a function, e.g.:
```
return pull_path_tuple(self._get_rslt_params(result, namespace))
```

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.

KSchopmeyer · 2016-06-09T01:04:58Z

+1 Changes made from comments plus some other issues I found.

andy-maier reviewed May 21, 2016
View reviewed changes

andy-maier reviewed May 23, 2016
View reviewed changes

andy-maier reviewed May 24, 2016
View reviewed changes

andy-maier assigned KSchopmeyer May 28, 2016

andy-maier added this to the 0.9.0 milestone May 28, 2016

KSchopmeyer force-pushed the issue#9_Pull_Operations branch from 6da1db7 to e38fc65 Compare May 30, 2016 01:06

KSchopmeyer changed the title ~~Initial definition of some of the pull operations.~~ fixes #9 implement DMTF Pull Operations with tests May 30, 2016

andy-maier mentioned this pull request May 30, 2016

cim_operations EnumerateInstances #329

Closed

KSchopmeyer force-pushed the issue#9_Pull_Operations branch from 1accd9b to 7683403 Compare June 6, 2016 00:54

KSchopmeyer force-pushed the issue#9_Pull_Operations branch from 7683403 to 152f073 Compare June 6, 2016 22:18

andy-maier removed rebase needed update needed labels Jun 7, 2016

andy-maier added the rebase needed label Jun 7, 2016

KSchopmeyer force-pushed the issue#9_Pull_Operations branch from 152f073 to bd8e08a Compare June 7, 2016 13:36

andy-maier removed the rebase needed label Jun 7, 2016

andy-maier added the review needed label Jun 7, 2016

andy-maier added update needed and removed review needed labels Jun 7, 2016

KSchopmeyer added the discussion needed label Jun 8, 2016

andy-maier removed the discussion needed label Jun 8, 2016

andy-maier mentioned this pull request Jun 8, 2016

Generate test_client yaml from within the WBEMConnection operation methods #351

Closed

KSchopmeyer mentioned this pull request Jun 8, 2016

Python 3 cim_operations.py, returns resource warning excessively #355

Closed

KSchopmeyer force-pushed the issue#9_Pull_Operations branch from bd8e08a to c35d540 Compare June 8, 2016 23:10

KSchopmeyer removed the update needed label Jun 8, 2016

andy-maier merged commit 4357769 into master Jun 9, 2016

andy-maier deleted the issue#9_Pull_Operations branch June 9, 2016 02:40

andy-maier removed the release: mandatory label Jun 10, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320

Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320

KSchopmeyer commented May 21, 2016

andy-maier May 21, 2016 •

edited by KSchopmeyer

KSchopmeyer commented May 23, 2016

andy-maier commented May 23, 2016

andy-maier May 23, 2016

KSchopmeyer commented May 23, 2016

andy-maier commented May 24, 2016

andy-maier May 24, 2016

andy-maier commented May 30, 2016

KSchopmeyer commented Jun 6, 2016

andy-maier commented Jun 6, 2016

andy-maier commented Jun 7, 2016

KSchopmeyer commented Jun 7, 2016

KSchopmeyer commented Jun 7, 2016

andy-maier commented Jun 7, 2016

andy-maier commented Jun 7, 2016 •

edited by KSchopmeyer

andy-maier commented Jun 7, 2016 •

edited by KSchopmeyer

andy-maier commented Jun 8, 2016 •

edited by KSchopmeyer

KSchopmeyer commented Jun 9, 2016

Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320

Fixes #9, Fixes #329: implement DMTF Pull Operations with tests #320

Conversation

KSchopmeyer commented May 21, 2016

andy-maier May 21, 2016 • edited by KSchopmeyer

Choose a reason for hiding this comment

KSchopmeyer commented May 23, 2016

andy-maier commented May 23, 2016

andy-maier May 23, 2016

Choose a reason for hiding this comment

KSchopmeyer commented May 23, 2016

andy-maier commented May 24, 2016

andy-maier May 24, 2016

Choose a reason for hiding this comment

andy-maier commented May 30, 2016

KSchopmeyer commented Jun 6, 2016

andy-maier commented Jun 6, 2016

andy-maier commented Jun 7, 2016

KSchopmeyer commented Jun 7, 2016

KSchopmeyer commented Jun 7, 2016

andy-maier commented Jun 7, 2016

andy-maier commented Jun 7, 2016 • edited by KSchopmeyer

andy-maier commented Jun 7, 2016 • edited by KSchopmeyer

andy-maier commented Jun 8, 2016 • edited by KSchopmeyer

KSchopmeyer commented Jun 9, 2016

andy-maier May 21, 2016 •

edited by KSchopmeyer

andy-maier commented Jun 7, 2016 •

edited by KSchopmeyer

andy-maier commented Jun 7, 2016 •

edited by KSchopmeyer

andy-maier commented Jun 8, 2016 •

edited by KSchopmeyer