Hi John

Thanks a lot for the review.

I have changed the comments as suggested by you. Please find the updated webrev.

https://cr.opensolaris.org/action/browse/caiman/nirmal27/7098861-2/webrev/

I have tested the fix with client and verified that client gets the manifest and profile file(s) as expected. You can find the details of testing in Manual testing file I provided.

/net/indiana-build.us.oracle.com//export/home/na210770/ai/7098861
/manual-Test

Thanks
Nirmal

On 03/21/12 23:13, John Fischer wrote:
Nirmal,

The code itself looks good and does what was discussed on the issue.
Comment on AI_database.py:

- Comment doesn't seem to match the code any longer

  659     # Save range values as either 1 or 0 for ORDERing purposes.
  660     # If either the MIN or MAX field in the db is non-NULL (if
  661     # unbounded, the MIN or MAX is NULL), then the "_val"
  662     # variable is set to 1. Otherwise, it is set to 0.
  663     # This allows for criteria that can be a range or an exact
  664     # value to be weighted equally during the ORDERing process
  665     # for manifests that have criteria set.
  666     # COALESCE returns a copy of its first non-NULL argument, or
  667     # NULL if all arguments are NULL.
  668     query_str = "SELECT name FROM manifests WHERE "





  669

- Should the comment be updated to explain the match_hostname situation?

  712                     # setup a clause like:
  713                     #    crit IS NULL OR \
  714                     #        is_in_list('crit', 'value', crit, 'None') == 
1
  715                     if crit == "hostname":
  716                         query_str += "( hostname IS NULL OR 
match_hostname" \
  717                                      "('" + sanitizeSQL(criteria[crit]) + 
\
  718                                      "', hostname, 0)  == 1) AND "
  719                     else:
  720                         query_str += "(" + crit + " IS NULL OR 
is_in_list" \
  721                                      "('" + crit + "', '" + \
  722                                      sanitizeSQL(criteria[crit]) + "', " 
+ \
  723                                      crit + ", 'None') == 1) AND "


Did you test this within an environment where the failure occurs (i.e.,
build and test an automated client)?

Thanks,

John

On 03/21/12 04:49 AM, Nirmal Agarwal wrote:
Hi all

Could I please get a code review for the following CR:

7098861 AI should find best possible match for manifest and profiles
when client hostname is fully qualified

Webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7098861

-- Source is pep8 clean.
-- ran pylint

slim_test result
/net/indiana-build.us.oracle.com//export/home/na210770/ai/7098861
/slim-test

Manual Tests Result :
/net/indiana-build.us.oracle.com//export/home/na210770/ai/7098861
/manual-Test

Thanks
Nirmal

_______________________________________________
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

Reply via email to