32515a6797b6e68daa3aa94c663760216b24dd67 lrnassar Thu Sep 3 13:30:26 2026 -0700 Usage stats cron fixes from code review. refs #38232 Apply the same db mismatch guard to assemblyStatsCron.py that generateUsageStats.py already had. Both files ask hgTracks for one assembly's default tracks and parse the answer off stderr, but only one of them checked that hgTracks answered about the assembly it was asked about. Without it, a repeat of the db= bug this ticket fixed would quietly fill the default track filter with another assembly's tracks and say nothing. Drop any database name that does not look like one before interpolating it into a shell command. The trimmed logs only ever carry real assembly names, so this is defence in depth rather than a live hole. Fix a comment in resolveHub that still described the lastOkTime fallback, which was replaced by mirror order earlier in this ticket. No change to the report. The row counts for the database usage and non-public hub tables were cut from 15 to 10 in the previous commit, which its message did not mention; that was intentional, to make room for the new GenArk and hubSpace sections. diff --git src/utils/qa/assemblyStatsCron.py src/utils/qa/assemblyStatsCron.py index f42c94eafb1..2a6dc42d1da 100755 --- src/utils/qa/assemblyStatsCron.py +++ src/utils/qa/assemblyStatsCron.py @@ -1,21 +1,22 @@ #07/20/19 #This was adapted from a jupyter notebook - hence lots of weird bash calls import datetime from collections import OrderedDict import getpass +import re import subprocess import os import urllib.parse def bash(cmd): """Run the cmd in bash subprocess""" try: rawBashOutput = subprocess.run(cmd, check=True, shell=True,\ stdout=subprocess.PIPE, universal_newlines=True, encoding="utf-8", errors="replace", stderr=subprocess.STDOUT) bashStdoutt = rawBashOutput.stdout except subprocess.CalledProcessError as e: raise RuntimeError("command '{}' return with error (code {}): {}".format(e.cmd, e.returncode, e.output)) return(bashStdoutt) @@ -214,31 +215,31 @@ try: return datetime.datetime.strptime(lastOkTime.split(" ")[0], '%Y-%m-%d') > lastMonth except ValueError: return False ambiguousHubIds = [] def resolveHub(hubId): """Return [hubUrl, shortLabel, machine] for a hub id, or None if no mirror can vouch for it""" candidates = [(machine, fields) for machine, fields in hubStatusByMirror.get(hubId, {}).items() if hubIsCurrent(fields[2])] if not candidates: return None if len(candidates) > 1: # Same id, different hubs. The track name we saw in the logs usually matches the right - # hub's shortLabel, so trust that before falling back to whoever refreshed most recently. + # hub's shortLabel, so trust that first. observedTrack = stripHubPrefix(bestTrackForHub[hubId][2]).lower() if hubId in bestTrackForHub else "" #Needs enough label to be evidence - a two letter shortLabel matches almost anything matching = [c for c in candidates if len(c[1][1]) >= 8 and (c[1][1].lower() in observedTrack or observedTrack.startswith(c[1][1].lower()[:20]))] if len(matching) == 1: candidates = matching #Otherwise fall through on mirror order, RR first, which is how this has always #resolved and matches a report written from the RR's point of view if len({normalizeHubUrl(c[1][0]) for c in candidates}) > 1: ambiguousHubIds.append(hubId) machine, fields = candidates[0] return [fields[0], fields[1], machine] resolvedHubs = {} #hubId -> [hubUrl, shortLabel, machine] for hubId in hubIdsSeen: @@ -301,54 +302,66 @@ with open(outputDir+'/dbCounts.tsv') as dbCountsForGenArk: for line in dbCountsForGenArk: dbName = line.split("\t")[0] hubId = hubIdFromName(dbName) if hubId is not None and hubId in resolvedHubs and hubCategory(resolvedHubs[hubId][0]) == 'genark': genArkAccessions.add(stripHubPrefix(dbName)) #The following section pulls out a list of default track for the top X assemblies for filtering defaultsFile = open(outputDir+"/defaults.txt", "w") #The head command can be expanded to be more inclusive if additional assembly defaults are finding their way onto the list bash("sort "+outputDir+'/dbCounts.tsv -rnk2 > '+outputDir+'/dbCountsTopSorted.tsv') topDbs = bash('head -n 10 '+outputDir+'/dbCountsTopSorted.tsv | cut -f1 -d "\t"').rstrip().split("\n") #Hub backed databases are skipped - hub ids differ between hgcentrals, so the track names #hgTracks hands back here would never match the ones seen in the logs. -topDbs = [db for db in topDbs if not db.startswith("hub_")][0:4] +#Assembly names are interpolated into a shell command below, so anything that does not look +#like one is dropped rather than passed to sh +topDbs = [db for db in topDbs + if not db.startswith("hub_") and re.match(r'^[A-Za-z0-9_.-]+$', db)][0:4] defaultTracks = set() for db in topDbs: #The following part queries hgTracks for each of the assemblies and extracts the list of defaults bash('echo '+db+' > '+outputDir+'/temp.txt') #Must run from the cgi-bin directory or hgTracks cannot find ../htdocs/urw-fonts and dies #before it ever logs the track list. The space before db= matters just as much - without it #cgiSpoof swallows the whole string and every assembly silently returns hg38's defaults. defaults = bashNoErrorCatch('cd /usr/local/apache/cgi-bin && HGDB_CONF=$HOME/.hg.conf.beta ' './hgTracks hgt.trackImgOnly=1 db='+db+' > /dev/null') #hgTracks writes "trackLog N db hgsid track:vis,track:vis" to stderr, split into ~800 byte #blocks so Apache does not chop the lines. Take every numbered block; the trailing #"trackLog position" line and the CGI_TIME/RESOURCE lines are not track lists. tracksForDb = [] + mismatch = None for defaultsLine in defaults: splitLine = defaultsLine.split(" ") if len(splitLine) > 4 and splitLine[0] == "trackLog" and splitLine[1].isdigit(): + #Check hgTracks answered about the assembly we asked for. Silently accepting + #another assembly's list is exactly how the defaults went wrong for two years. + if splitLine[2] != db: + mismatch = splitLine[2] + tracksForDb = [] + break tracksForDb.extend(splitLine[4].split(",")) if not tracksForDb: - reportWarnings.append("WARNING: hgTracks returned no default track list for "+db+ - ", its tracks are not filtered out of the non-default list below.") + reason = "it reported "+mismatch+" instead" if mismatch else "it returned no track list" + reportWarnings.append("WARNING: asked hgTracks for the default tracks of "+db+" and "+ + reason+", so its tracks are not filtered out of the non-default " + "list below.") continue for track in tracksForDb: track = track.rsplit(":", 1)[0] #drop the trailing visibility if track: defaultsFile.write(db+"\t"+track+"\n") defaultTracks.add((db, track)) defaultsFile.write(db+"\t"+"cytoBand"+"\n") defaultTracks.add((db, "cytoBand")) defaultsFile.close() ############################################# ##### Build the report, section by section ## #############################################