Index: lnt/lnttool/admin.py =================================================================== --- lnt/lnttool/admin.py +++ lnt/lnttool/admin.py @@ -1,5 +1,6 @@ #!/usr/bin/env python import click +from .common import submit_options def _load_dependencies(): @@ -286,11 +287,8 @@ @_pass_config @click.argument("datafiles", nargs=-1, type=click.Path(exists=True), required=True) -@click.option("--update-machine", is_flag=True, help="Update machine fields") -@click.option("--merge", default="replace", show_default=True, - type=click.Choice(['reject', 'replace', 'merge']), - help="Merge strategy when run already exists") -def action_post_run(config, datafiles, update_machine, merge): +@submit_options +def action_post_run(config, datafiles, commit, select_machine, merge): """Submit report files to server.""" _check_auth_token(config) @@ -301,7 +299,7 @@ url = ('{lnt_url}/api/db_{database}/v4/{testsuite}/runs' .format(**config.dict)) url_params = { - 'update_machine': 1 if update_machine else 0, + 'select_machine': select_machine, 'merge': merge, } response = config.session.post(url, params=url_params, data=data, Index: lnt/lnttool/common.py =================================================================== --- lnt/lnttool/common.py +++ lnt/lnttool/common.py @@ -6,8 +6,9 @@ def submit_options(func): func = click.option("--commit", is_flag=True, help="actually commit the data")(func) - func = click.option("--update-machine", is_flag=True, - help="Update machine fields")(func) + func = click.option("--select-machine", default='match', + type=click.Choice(['match', 'update', 'split']), + help="How to select and create missing machine")(func) func = click.option("--merge", default="replace", show_default=True, type=click.Choice(['reject', 'replace', 'append']), help="Merge strategy when run already exists")(func) Index: lnt/lnttool/import_data.py =================================================================== --- lnt/lnttool/import_data.py +++ lnt/lnttool/import_data.py @@ -23,7 +23,7 @@ @submit_options def action_import(instance_path, files, database, output_format, show_sql, show_sample_count, show_raw_result, testsuite, verbose, - quiet, no_email, no_report, commit, update_machine, merge): + quiet, no_email, no_report, commit, select_machine, merge): """import test data into a database""" import contextlib import lnt.server.instance @@ -44,8 +44,8 @@ result = lnt.util.ImportData.import_and_report( config, database, db, file_name, output_format, testsuite, commit, show_sample_count, - no_email, no_report, updateMachine=update_machine, - mergeRun=merge) + no_email, no_report, select_machine=select_machine, + merge_run=merge) success &= result.get('success', False) if quiet: @@ -55,8 +55,7 @@ pprint.pprint(result) else: lnt.util.ImportData.print_report_result(result, sys.stdout, - sys.stderr, - verbose) + sys.stderr, verbose) if not success: raise SystemExit(1) Index: lnt/lnttool/main.py =================================================================== --- lnt/lnttool/main.py +++ lnt/lnttool/main.py @@ -181,7 +181,7 @@ @submit_options @click.option("--verbose", "-v", is_flag=True, help="show verbose test results") -def action_submit(url, files, commit, update_machine, merge, verbose): +def action_submit(url, files, commit, select_machine, merge, verbose): """submit a test report to the server""" from lnt.util import ServerUtil import lnt.util.ImportData @@ -194,8 +194,8 @@ "your results will not be saved at the server.") files = ServerUtil.submitFiles(url, files, commit, verbose, - updateMachine=update_machine, - mergeRun=merge) + select_machine=select_machine, + merge_run=merge) for submitted_file in files: if verbose: lnt.util.ImportData.print_report_result( Index: lnt/lnttool/viewcomparison.py =================================================================== --- lnt/lnttool/viewcomparison.py +++ lnt/lnttool/viewcomparison.py @@ -84,9 +84,11 @@ # Import the two reports. with contextlib.closing(config.get_database('default')) as db: r = import_and_report(config, 'default', db, report_a, '', - testsuite, commit=True, updateMachine=True) + testsuite, commit=True, + select_machine='match') import_and_report(config, 'default', db, report_b, '', - testsuite, commit=True, updateMachine=True) + testsuite, commit=True, + select_machine='match') # Dispatch another thread to start the webbrowser. comparison_url = '%s/v4/nts/2?compare_to=1' % (url,) Index: lnt/server/db/testsuitedb.py =================================================================== --- lnt/server/db/testsuitedb.py +++ lnt/server/db/testsuitedb.py @@ -774,13 +774,55 @@ return None - def _getOrCreateMachine(self, machine_data, forceUpdate): + def _getIncompatibleFields(self, existing_machine, new_machine): + incompatible_fields = set() + for field in self.machine_fields: + existing_value = existing_machine.get_field(field) + new_value = new_machine.get_field(field) + if new_value is None or existing_value == new_value: + continue + if existing_value is not None: + incompatible_fields.add(field.name) + existing_parameters = existing_machine.parameters + for key, new_value in new_machine.parameters.items(): + existing_value = existing_parameters.get(key, None) + if new_value is None or existing_value == new_value: + continue + if existing_value is not None: + incompatible_fields.add(key) + return incompatible_fields + + def _updateMachine(self, existing_machine, new_machine): + for field in self.machine_fields: + new_value = new_machine.get_field(field) + if new_value is None: + continue + existing_machine.set_field(field, new_value) + parameters = existing_machine.parameters + for key, new_value in new_machine.parameters.items(): + if new_value is None and parameters.get(key, None) is not None: + continue + parameters[key] = new_value + existing_machine.parameters = parameters + + def _getOrCreateMachine(self, machine_data, select_machine): """ - _getOrCreateMachine(data, forceUpdate) -> Machine + _getOrCreateMachine(data, select_machine) -> Machine Add or create (and insert) a Machine record from the given machine data (as recorded by the test interchange format). + + select_machine strategies: + 'match': Abort if the existing machine doesn't match the new machine + data. + 'update': Update the existing machine in cases where the new machine + data doesn't match the existing data. + 'split': On parameter mismatch create a new machine with a `$NN` suffix + added, or choose an existing compatible machine with such a + suffix. """ + assert select_machine == 'match' or select_machine == 'update' \ + or select_machine == 'split' # Convert the machine data into a machine record. machine_parameters = machine_data.copy() @@ -797,46 +839,36 @@ .filter(self.Machine.name == name) \ .order_by(self.Machine.id.desc()) \ .all() + # No existing machine? Add one. if len(existing_machines) == 0: self.add(machine) return machine - - existing = existing_machines[0] - - # Unfortunately previous LNT versions allowed multiple machines - # with the same name to exist, so we should choose the one that - # matches best. - if len(existing_machines) > 1: - for m in existing_machines: - if m.parameters == machine.parameters: - existing = m - break - - # Check and potentially update existing machine. - # Parameters that were previously unset are added. If a parameter - # changed then we update or abort depending on `forceUpdate`. - for field in self.machine_fields: - existing_value = existing.get_field(field) - new_value = machine.get_field(field) - if new_value is None or existing_value == new_value: - continue - if existing_value is None or forceUpdate: - existing.set_field(field, new_value) - else: + # Search for a compatible machine. + existing_machine = None + incompatible_fields_0 = [] + for m in existing_machines: + incompatible_fields = self._getIncompatibleFields(m, machine) + if len(incompatible_fields) == 0: + existing_machine = m + break + if len(incompatible_fields_0) == 0: + incompatible_fields_0 = incompatible_fields + # All existing machines are incompatible? + if existing_machine is None: + if select_machine == 'split': + # Add a new machine. + self.add(machine) + return machine + if select_machine == 'match': raise MachineInfoChanged("'%s' on machine '%s' changed." % - (field.name, name)) - existing_parameters = existing.parameters - for key, new_value in machine.parameters.items(): - existing_value = existing_parameters.get(key, None) - if new_value is None or existing_value == new_value: - continue - if existing_value is None or forceUpdate: - existing_parameters[key] = value + (', '.join(incompatible_fields_0), name)) else: - raise MachineInfoChanged("'%s' on machine '%s' changed." % - (key, name)) - existing.parameters = existing_parameters - return existing + assert select_machine == 'update' + # Just pick the first and update it below. + existing_machine = existing_machines[0] + + self._updateMachine(existing_machine, machine) + return existing_machine def _getOrCreateOrder(self, run_parameters): """ @@ -1009,19 +1041,19 @@ else: sample.set_field(field, value) - def importDataFromDict(self, data, commit, config, updateMachine, - mergeRun): + def importDataFromDict(self, data, commit, config, select_machine, + merge_run): """ - importDataFromDict(data, commit, config, updateMachine, mergeRun) + importDataFromDict(data, commit, config, select_machine, merge_run) -> Run (or throws ValueError exception) Import a new run from the provided test interchange data, and return the constructed Run record. May throw ValueError exceptions in cases like mismatching machine data or duplicate run submission with - mergeRun == 'reject'. + merge_run == 'reject'. """ - machine = self._getOrCreateMachine(data['machine'], updateMachine) - run = self._getOrCreateRun(data['run'], machine, mergeRun) + machine = self._getOrCreateMachine(data['machine'], select_machine) + run = self._getOrCreateRun(data['run'], machine, merge_run) self._importSampleValues(data['tests'], run, commit, config) return run Index: lnt/server/ui/api.py =================================================================== --- lnt/server/ui/api.py +++ lnt/server/ui/api.py @@ -288,11 +288,11 @@ """Add a new run into the lnt database""" db = request.get_db() data = request.data - updateMachine = request.values.get('update_machine', False) + select_machine = request.values.get('select_machine', 'match') merge = request.values.get('merge', 'replace') result = lnt.util.ImportData.import_from_string( current_app.old_config, g.db_name, db, g.testsuite_name, data, - updateMachine=updateMachine, mergeRun=merge) + select_machine=select_machine, merge_run=merge) error = result['error'] if error is not None: Index: lnt/server/ui/templates/submit_run.html =================================================================== --- lnt/server/ui/templates/submit_run.html +++ lnt/server/ui/templates/submit_run.html @@ -20,10 +20,18 @@
-

Update Machine:
- + + + +
+ +

Merge with existing runs:
+

Index: lnt/server/ui/views.py =================================================================== --- lnt/server/ui/views.py +++ lnt/server/ui/views.py @@ -102,8 +102,8 @@ input_file = request.files.get('file') input_data = request.form.get('input_data') commit = int(request.form.get('commit', 0)) != 0 - updateMachine = int(request.form.get('update_machine', 0)) != 0 - merge = request.form.get('merge', 'replace') + select_machine = request.form.get('select_machine', 'match') + merge_run = request.form.get('merge', 'replace') if input_file and not input_file.content_length: input_file = None @@ -143,7 +143,7 @@ result = lnt.util.ImportData.import_from_string( current_app.old_config, g.db_name, db, g.testsuite_name, data_value, - commit=commit, updateMachine=updateMachine, mergeRun=merge) + commit=commit, select_machine=select_machine, merge_run=merge_run) # It is nice to have a full URL to the run, so fixup the request URL # here were we know more about the flask instance. Index: lnt/tests/builtintest.py =================================================================== --- lnt/tests/builtintest.py +++ lnt/tests/builtintest.py @@ -78,7 +78,7 @@ self.log("submitting result to %r" % (config.submit_url,)) server_report = ServerUtil.submitFile( config.submit_url, report_path, commit, config.verbose, - updateMachine=config.update_machine, mergeRun=config.merge) + select_machine=config.select_machine, merge_run=config.merge) else: server_report = lnt.util.ImportData.no_submit() if server_report: Index: lnt/util/ImportData.py =================================================================== --- lnt/util/ImportData.py +++ lnt/util/ImportData.py @@ -15,7 +15,7 @@ def import_and_report(config, db_name, db, file, format, ts_name, commit=False, show_sample_count=False, disable_email=False, disable_report=False, - updateMachine=False, mergeRun='replace'): + select_machine='match', merge_run='replace'): """ import_and_report(config, db_name, db, file, format, ts_name, [commit], [show_sample_count], @@ -35,6 +35,10 @@ 'import_file': file, } + if select_machine not in ('match', 'update', 'split'): + result['error'] = "select_machine must be 'match', 'update' or 'split'" + return result + ts = db.testsuite.get(ts_name) if ts is None: result['error'] = "Unknown test suite '%s'!" % ts_name @@ -92,8 +96,8 @@ return result run = ts.importDataFromDict(data, commit, config=db_config, - updateMachine=updateMachine, - mergeRun=mergeRun) + select_machine=select_machine, + merge_run=merge_run) except KeyboardInterrupt: raise except Exception as e: @@ -101,8 +105,9 @@ result['error'] = "import failure: %s" % e.message result['message'] = traceback.format_exc() if isinstance(e, lnt.server.db.testsuitedb.MachineInfoChanged): - result['message'] += '\n\nNote: Use --update-machine to update ' \ - 'the existing machine information.\n' + result['message'] += \ + '\n\nNote: Use --select-machine=update to update ' \ + 'the existing machine information.\n' return result # If the import succeeded, save the import path. @@ -163,7 +168,8 @@ shadow_db, file, format, ts_name, commit, show_sample_count, disable_email, disable_report, - updateMachine) + select_machine=select_machine, + merge_run=merge_run) # Append the shadow result to the result. result['shadow_result'] = shadow_result @@ -313,7 +319,7 @@ def import_from_string(config, db_name, db, ts_name, data, commit=True, - updateMachine=False, mergeRun='replace'): + select_machine='match', merge_run='replace'): # Stash a copy of the raw submission. # # To keep the temporary directory organized, we keep files in @@ -342,5 +348,5 @@ result = lnt.util.ImportData.import_and_report( config, db_name, db, path, '', ts_name, commit, - updateMachine=updateMachine, mergeRun=mergeRun) + select_machine=select_machine, merge_run=merge_run) return result Index: lnt/util/ServerUtil.py =================================================================== --- lnt/util/ServerUtil.py +++ lnt/util/ServerUtil.py @@ -29,13 +29,13 @@ sys.stderr.write(message + '\n') -def submitFileToServer(url, file, commit, updateMachine, mergeRun): +def submitFileToServer(url, file, commit, select_machine, merge_run): with open(file, 'rb') as f: values = { 'input_data': f.read(), 'commit': "1" if commit else "0", - 'update_machine': "1" if updateMachine else "0", - 'merge': mergeRun, + 'select_machine': select_machine, + 'merge': merge_run, } headers = {'Accept': 'application/json'} data = urllib.urlencode(values) @@ -63,8 +63,8 @@ return reply -def submitFileToInstance(path, file, commit, updateMachine=False, - mergeRun='replace'): +def submitFileToInstance(path, file, commit, select_machine='match', + merge_run='replace'): # Otherwise, assume it is a local url and submit to the default database # in the instance. instance = lnt.server.instance.Instance.frompath(path) @@ -75,26 +75,27 @@ raise ValueError("no default database in instance: %r" % (path,)) return lnt.util.ImportData.import_and_report( config, db_name, db, file, format='', ts_name='nts', - commit=commit, updateMachine=updateMachine, mergeRun=mergeRun) + commit=commit, select_machine=select_machine, merge_run=merge_run) -def submitFile(url, file, commit, verbose, updateMachine=False, - mergeRun='replace'): +def submitFile(url, file, commit, verbose, select_machine='match', + merge_run='replace'): # If this is a real url, submit it using urllib. if '://' in url: - result = submitFileToServer(url, file, commit, updateMachine, mergeRun) + result = submitFileToServer(url, file, commit, select_machine, + merge_run) else: - result = submitFileToInstance(url, file, commit, updateMachine, - mergeRun) + result = submitFileToInstance(url, file, commit, select_machine, + merge_run) return result -def submitFiles(url, files, commit, verbose, updateMachine=False, - mergeRun='replace'): +def submitFiles(url, files, commit, verbose, select_machine='match', + merge_run='replace'): results = [] for file in files: - result = submitFile(url, file, commit, verbose, updateMachine, - mergeRun) + result = submitFile(url, file, commit, verbose, + select_machine=select_machine, merge_run=merge_run) if result: results.append(result) return results Index: tests/lnttool/Inputs/compile_submission_machine_diff_split.json =================================================================== --- /dev/null +++ tests/lnttool/Inputs/compile_submission_machine_diff_split.json @@ -0,0 +1,29 @@ +{ + "Machine": { + "Info": { + "hw.activecpu": "2", + "machdep.cpu.vendor": "AMD" + }, + "Name": "some-compile-suite-machine" + }, + "Run": { + "End Time": "2017-07-06 15:37:08", + "Start Time": "2017-07-06 15:05:23", + "Info": { + "__report_version__": "1", + "run_order": "663405", + "tag": "compile" + } + }, + "Tests": [ + { + "Data": [ + 13.601326, + 13.411566, + 13.490528 + ], + "Info": {}, + "Name": "compile.build/Adium-1.5.7(config='Debug',j=1).user" + } + ] +} Index: tests/lnttool/submit.shtest =================================================================== --- tests/lnttool/submit.shtest +++ tests/lnttool/submit.shtest @@ -108,7 +108,7 @@ # CHECK-ERRORS: error: lnt server: import failure: 'hw.activecpu' on machine 'some-compile-suite-machine' changed. # ... # CHECK-ERRORS: MachineInfoChanged: 'hw.activecpu' on machine 'some-compile-suite-machine' changed. -# CHECK-ERRORS: Note: Use --update-machine to update the existing machine information. +# CHECK-ERRORS: Note: Use --select-machine=update to update the existing machine information. @@ -128,7 +128,7 @@ # CHECK-MACHINEDIFF: Results available at: http://localhost:9091/db_default/v4/compile/7 # Test updating existing machine -lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_reject.json" --update-machine -v > "${OUTPUT_DIR}/submit_compile_machine_update.txt" +lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_reject.json" --select-machine=update -v > "${OUTPUT_DIR}/submit_compile_machine_update.txt" # RUN: FileCheck %s --check-prefix=CHECK-UPDATEMACHINE < %t.tmp/submit_compile_machine_update.txt # # CHECK-UPDATEMACHINE: Imported Data @@ -141,3 +141,18 @@ # CHECK-UPDATEMACHINE: ---------------- # CHECK-UPDATEMACHINE: PASS : 9 # CHECK-UPDATEMACHINE: Results available at: http://localhost:9091/db_default/v4/compile/8 + +# Test creationg of new machines on information mismatch in split mode. +lnt submit "http://localhost:9091/db_default/v4/compile/submitRun" --commit "${INPUTS}/compile_submission_machine_diff_split.json" --select-machine=split -v > "${OUTPUT_DIR}/submit_compile_machine_split.txt" +# RUN: FileCheck %s --check-prefix=CHECK-SPLITMACHINE < %t.tmp/submit_compile_machine_split.txt +# +# We should have added a new machine: +# CHECK-SPLITMACHINE: Imported Data +# CHECK-SPLITMACHINE: ------------- +# CHECK-SPLITMACHINE: Added Machines: 1 +# CHECK-SPLITMACHINE: Added Runs : 1 +# +# CHECK-SPLITMACHINE: Results +# CHECK-SPLITMACHINE: ---------------- +# CHECK-SPLITMACHINE: PASS : 5 +# CHECK-SPLITMACHINE: Results available at: http://localhost:9091/db_default/v4/compile/9 Index: tests/server/db/search.py =================================================================== --- tests/server/db/search.py +++ tests/server/db/search.py @@ -45,8 +45,8 @@ None, 'default', self.db, f.name, format='', ts_name='nts', commit=True, show_sample_count=False, disable_email=True, - disable_report=True, updateMachine=False, - mergeRun='reject') + disable_report=True, select_machine='match', + merge_run='reject') assert result.get('success', False)