Nirmal,
Looks good. Can you change the name of the non match function?
554 def test_match_fqhn_value_list(self):
To perhaps something like:
def test_no_match_fqhn_value(self):
As this will then match what is done within the is_in_list class above.
Having list in the name is fine too but the no_match makes it a little
clearer.
No second review from me is needed.
Thanks,
John
On 03/28/12 10:24 AM, Nirmal Agarwal wrote:
Hi all
Please find the latest webrev for CR 7098861 :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7098861-3/webrev/
I have added a test case for 'match_hostname' function which I
introduced.
Please let me know your comments.
Thanks
Nirmal
On 3/22/2012 9:47 PM, John Fischer wrote:
Nirmal,
Looks good to me.
John
On 03/22/12 02:32 AM, Nirmal Agarwal wrote:
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
