Clay and I did a code walk through of file: publish_manifest.py The comments are below.
Joe Clay Baenziger wrote: > Hello all, > I have written a prototype A/I webserver to serve files and A/I > manifests for the November release. The code and some example > databases are available at: > http://cr.opensolaris.org/~clayb/AI_server/AI_server.tgz > > This is only the webserver, however, and not the associated > manifest publishing utility to populate the server. I have included > databases and files to test various aspects, however: > AI.db.populated (which has a test with two manifests having overlapping > criteria -- shouldn't be allowed by publish_manifest and > generates an error) > AI.db.empty (which only has a schema but no entries - to test serving > simply a default manifest) > AI.db.populated2(this shows that the webserver and the DB format is > extensibility allowing extra criteria to be added modularly > though we will not be supporting such features for > November) > > To work with these databases rename the one to work with, as AI.db in the > AI_server directory. > > Similarly, you'll see I've two temporary files manifest[12].xml but no > manifest3.xml. This should generate a 404 Not Found error, if one matches > the correct criteria in AI.db.populated. Lastly, to serve the solaris.zlib > files there's also a /ai-files server path which serves out of AI_files. > > Next, try the following paths to request manifests: > /manifest.xml > /manifest.html > Both should accept data via POST with key value pairs such as: > MAC=c0ffee01;Ip=192168001002;arch=sun4v > > And lastly, to run the server run 'python webserver.py', you'll need the > CherryPy (http://cherrypy.org) and PySQLite2 (http://pysqlite.org) > Python modules. > > Thank you, > Clay > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= The code might be more readable if blank lines were added throughout. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= how to get _AIRoot 313 currentMD5 = md5.new(lxml.etree.tostring(files._AIRoot, and other lines... 332 newManifest.writelines(lxml.etree.tostring(files._smfDict[key], 333 pretty_print=True, encoding=unicode)) should have accessor around _AIRoot and _smfDict +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= Add comment above code block of code like at line 41->48 dnd 51->57 describing what is expected. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 50 files.service(args[0]) 58 files.database(os.path.join(files.service(), "AI.db")) 59 files.database().verifyDBStructure() Should flag they they will exit if errors are encountered. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= Explain bailout conditions (as can happen on the four lines 578,583, 585, 588) +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= If ai manifest is passed in on the command line... 62 if options.ai: The manifest if available is ignored? I think this should be clearify with comments. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 69 except SystemExit: What is causing this exception? +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 66 if files.findAIfromCriteria(): Is this a bug? Note From Clay: Move line 63->71 Note From Clay: Move line 65->69 +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 76 if options.criteria and files.manifestName() == "default.xml": Remove options.criteria or check for criteria in the criteria manifes +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 79 files._smfDict['commandLine'] = \ Need comment or better design to prevent or flag a collision can occur if SC name="commandLine"? +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 298 except: This should this be a: except IOError, e: +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 303 Compares src and dst manifests to ensure they are the same, copies new 304 manifest into place if no existing dst manifest, corrects permissions and 305 ownership Comment doesn't match code exactly. Also not this block of code will not be entered for a default manifest because os.path.exists will fail. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= Remove os.path.basename from: 307 if os.path.exists(os.path.join(files.service(), "AI_data", 308 os.path.basename(files.manifestName()))): should be: 307 if os.path.exists(os.path.join(files.service(), "AI_data", 308 files.manifestName())): same thing on the following lines: 243 queryStr += "'" + AIdb.sanitizeSQL(os.path.basename( 310 os.path.basename(files.manifestName())), "r") 320 os.path.basename(files.manifestName())), "w") 340 os.path.basename(files.manifestName())), 0555) 343 os.path.basename(files.manifestName())), 0, 0) +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= Line 307 there abouts What happens if the file exists and does not differ you need not write the manifest again. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= Line 323 -> 335 is there a better way than writelines to write the XML? Also xml string you are attempting to write is not verified. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 84 def checkCriteria(files): This would be a difficult function to maintain. It is not easily determined what the algorithm is. checkCriteria is large. Does it make sense to break checkCriteria into multiple functions? Comment what collisions is used for. Check it there isn't a better way to manage this than using dict in Python. +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-= 237 def insertSQL(files): Improve commenting. 246: Check to see if manifes is already in database 258: This is run if the criteria is a range criteria 271: This is run for single value criteria 278: This is run if the criteria manifest does not specify this criteria 283: Strip the trailing comma and close the parentheses
