Keith,
Thanks. My testing was incorrect. I thought that I had tested a new
client with an old
service with a manifest when in reality I tested a new client with a new
service that had
a manifest. Ugh. So I didn't catch that the code had a huge bug in
it. Totally embarrassing.
Anyhow, thanks for pointing out the known_criteria issue below. Other
responses in-line.
I'll update the webrev when I get things working correctly.
Thanks,
John
On 02/ 3/11 11:30 AM, Keith Mitchell wrote:
Hi John,
A few questions:
In ai_get_manifest.py, the signature for ai_get_http_file has
*nv_pairs. Based on the usage, that should simply be "nv_pairs"
without the asterisk (and the code below should replace "nv_pairs[0]"
with "nv_pairs").
Sure. Old code just being used from before but that makes it much cleaner.
ai_get_requested_criteria_list: The comment on lines 730-745 should
probably be moved up to this function instead, to be more clear.
Beyond that, what if the criteria file was hand-edited by an admin and
has extra spaces, e.g., <Criteria Name="abc" />? (Or is the
XML normalized at some point before this?) Additionally, the docstring
mentions that it returns 0 for success, and -1 for failure, but the
function never directly returns either of those. (probably best to
just return the required_criteria list and omit the return value)
Right. I've moved the comment up to the function that actually does the
"parsing" if you
can call it that. You are correct that this doesn't really parse the
file correctly. This part
of the code was sucked back into the tree from before my AI webserver
changes yesterday.
Essentially, it is doing what it did before. It really needs to be
replaced with an XML
parser.
With that said. The criteria file is not actually coming from a system
administrator directly.
It is coming from the webserver. It can only get into the webserver if
it has passed through
the installadm add-manifest command. Thus it has been parsed for
correctness at that
end. So we are relatively safe here.
I corrected the docstring.
753-755: ai_criteria_known is not defined in this scope - should this
be "known_criteria"?
yep. That is the testing issue.
752 - 758: Pythonization suggestion:
for criteria in criteria_required:
if known_criteria.get(criteria, None):
ai_crit_response[criteria] = known_criteria[criteria]
That does look nicer.
- Keith
On 02/ 3/11 10:05 AM, John Fischer wrote:
All and Sue (specifically for the AI_database change),
After I delivered the AI webserver design project Mary found that the
new ISOs could
not get a manifest from build 157 and 158 cherrypy webservers. The
failure was because
a new bug fix went into the AI_database.py code that was not
anticipated.
The webrev is located at:
http://cr.opensolaris.org/~johnfisc/7016997-get-manifest/
The solution is 2 fold. First, the database criteria list needs to
be checked within the
findManifest() function prior to it being used. Secondly, the
ai_get_manifest.py compatibility
code needs to be enhanced to do the handshaking it used to do prior
to the protocol
changes. Essentially, the compatibility re-introduces the handshake
code.
Thanks,
John
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
