ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

computers: the machine number identifies the machine, not the PC

Browse Source 
A bay reporting machinenumber 3015 got a 500 from the collector every five
minutes since it was imaged, and would have forever: the reported number was
written to the PC's own assets.assetnumber, which is uniquely indexed and
already held by machine 3015, so the insert failed with "Duplicate entry '3015'
for key 'ix_assets_assetnumber'" and the entire report was discarded. Operating
system, boot time, applications, printers and access protocols never landed.
Every retry did the same thing, so there was no path out of it.

A new PC now takes its hostname as its asset number, which is what the data
already shows: of 289 computers none has a numeric asset number and 214 use
their hostname. An existing PC's asset number is left alone; overwriting it
renamed the PC onto the machine's identifier, changing how that PC is
identified everywhere else.

The machine number instead does what it was collected for. It resolves the
machine and links the PC to it with a 'controls' relationship carrying a
collector:machine origin label, the same discipline the printer and
measuring-tool links use, so a link made by hand is never archived by a
collector push. Reporting a different machine archives this PC's previous link;
a machine ShopDB does not know is reported as a warning rather than invented.

When another PC was already linked to that machine it has been replaced. The
old link is archived rather than deleted, so which PC ran a machine in a given
month remains answerable, and an alert goes out by email and webhook. The
retired PC's status is deliberately not changed: the collector cannot tell
whether it was shelved, sent for repair or re-imaged for another bay, and
guessing would overwrite what a person set.
This commit is contained in:
cproudlock
2026-08-10 14:24:55 -04:00
parent b8398a36eb
commit 9512b0bdb3
3 changed files with 377 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
CHANGELOG.md
View File

@@ -12,6 +12,19 @@ ADR-007 and ADR-002.
### Fixed
- A shop-floor PC that reported the machine number of a machine ShopDB already
knew got a 500 from the collector, on every report, forever. The reported
machine number was being written to the PC's own `assets.assetnumber`, which
is uniquely indexed and already held by the machine, so the insert failed with
a duplicate-key error and the whole report was lost: operating system, boot
time, applications, printers and access protocols never landed. The machine
number now identifies the machine only. A new PC takes its hostname as its
asset number, which is what the existing data already does, and an existing
PC's asset number is never overwritten.
- A backup export containing a value with an empty right-hand side (`Name=`)
failed to parse, and with it the whole file, so that machine could never be
backed up. The form is not strictly legal but occurs in real exports. It is
now read as an empty string, preserving the value name.
- The 3D parts kiosk label prefix never appeared on the kiosk. The kiosk runs
logged out, and an unauthenticated read of a setting is limited to an
allowlist the key was not on, so the kiosk got a 404 and fell back to no
@@ -35,6 +48,17 @@ ADR-007 and ADR-002.
### Added
- A PC reporting a machine number is now linked to that machine. Until now the
number was collected and then discarded, so a replaced PC never took over its
bay: the retired PC kept the link and everything that walks PC to machine, the
warranty machine column and the DNC info card among them, pointed at hardware
that had been pulled out. The link is a `controls` relationship carrying a
`collector:machine` origin label, so a link made by hand is never touched.
Superseding a PC archives the old link rather than deleting it, keeping the
history of which PC ran a machine and when, and raises an alert by email and
webhook. The retired PC's status is deliberately left alone: the collector
cannot tell whether it was shelved, sent for repair or re-imaged for another
bay, so a person decides.
- Plugin contract 0.16.0: `get_settings_defaults()` lets a plugin declare the
settings it owns (key, default, type, category, description, and whether an
unauthenticated caller may read it). See `docs/PLUGIN-HOOKS.md`.

163
plugins/computers/plugin.py
View File

@@ -28,6 +28,12 @@ PRINTER_LINK_ORIGIN = 'collector:printers'
# by a collector push, so hand-made tool links survive.
MEASURINGTOOL_LINK_ORIGIN = 'collector:measuringtool'
# Marker stamped on the PC->machine "controls" link built from the reported
# machine number. Same discipline as the two above: only rows carrying this
# label are archived by a collector push, so a link made by hand is never
# touched.
MACHINE_LINK_ORIGIN = 'collector:machine'
class ComputersPlugin(BasePlugin):
"""
@@ -153,8 +159,15 @@ class ComputersPlugin(BasePlugin):
if not hostname:
raise ValueError('hostname is required')
# Machine number is the business identifier (Asset.assetnumber). Skip
# the imaging-time placeholder '9999' and fall back to hostname.
# The machine number identifies the MACHINE, not this PC. It is
# reported so the PC can be related to its machine; it is deliberately
# NOT used as the PC's assetnumber. assets.assetnumber is uniquely
# indexed and the machine already owns that value, so assigning it here
# raised "Duplicate entry '3015' for key 'ix_assets_assetnumber'" and
# returned 500 to the bay - forever, since every retry did the same
# thing. The convention this restores is what the data already shows:
# of 289 computers, none has a numeric assetnumber and 214 use their
# hostname.
machinenumber = (payload.get('machinenumber') or '').strip()
if machinenumber in ('', '9999'):
machinenumber = None
@@ -169,7 +182,7 @@ class ComputersPlugin(BasePlugin):
atype = AssetType.query.filter_by(assettype='computer').first()
# statusid=1 is the first seeded asset status ("In Use"); a
# collector-discovered PC is by definition in use.
asset = Asset(assetnumber=machinenumber or hostname,
asset = Asset(assetnumber=hostname,
assettypeid=atype.assettypeid, statusid=1)
db.session.add(asset)
db.session.flush()
@@ -177,8 +190,10 @@ class ComputersPlugin(BasePlugin):
db.session.add(comp)
db.session.flush()
action = 'created'
elif machinenumber and comp.asset:
comp.asset.assetnumber = machinenumber
# NOTE: an existing PC's assetnumber is left alone. Overwriting it with
# the machine number renamed the PC onto the machine's identifier, which
# either collided with the unique index or silently changed how that PC
# is identified everywhere else.
comp.lastreporteddate = datetime.now(timezone.utc).replace(tzinfo=None)
if payload.get('lastboottime'):
@@ -277,6 +292,9 @@ class ComputersPlugin(BasePlugin):
# Remote-access protocol sync (only when the payload carried the key).
accessprotocols = self._sync_access_protocols(comp, payload, warnings)
# PC -> machine link from the reported machine number.
machinelinks = self._sync_machine_link(comp, machinenumber, warnings)
# Printer relationship sync (only when the payload carried printer data).
printerlinks = self._sync_printer_links(comp.asset, payload, warnings)
@@ -297,6 +315,7 @@ class ComputersPlugin(BasePlugin):
'measuringtoollinks': measuringtoollinks,
'measuringtoollinkcount': len(measuringtoollinks),
'accessprotocols': accessprotocols,
'machinelinks': machinelinks,
},
}
@@ -494,6 +513,140 @@ class ComputersPlugin(BasePlugin):
# -- measuring-tool sync -----------------------------------------------
def _sync_machine_link(self, comp, machinenumber, warnings):
"""Link a PC to the machine it drives, from the reported machine number.
Until this existed the machine number was collected and then discarded,
so a replaced PC never took over its bay: the retired PC kept the link
and the new one got none. Everything that walks PC->machine (the
warranty machine column, the DNC info card) therefore pointed at
hardware that had been pulled out.
ARCHIVES, never deletes. A superseded link stays with isactive=False so
"which PC ran 3015 in June" is still answerable. Status on the old PC is
deliberately NOT changed: the collector cannot tell whether it was
shelved, broken or re-imaged for another bay, and guessing would
overwrite whatever a person deliberately set.
"""
from shopdb.api import AssetRelationship, RelationshipType, Asset
pcasset = comp.asset if comp else None
if not machinenumber or not pcasset:
return []
controls = RelationshipType.query.filter_by(
relationshiptype='controls').first()
if not controls:
warnings.append("'controls' relationship type missing; "
'run flask seed reference-data')
return []
machine = Asset.query.filter(
Asset.assetnumber.ilike(machinenumber),
Asset.isactive.is_(True)).first()
if not machine:
# Reported a machine ShopDB does not know. Warn rather than invent
# an asset: a mistyped number would create a machine nobody can
# account for.
warnings.append(
'no asset for machine number {!r}; PC not linked'.format(
machinenumber))
return []
if machine.assetid == pcasset.assetid:
return []
# This PC's own collector links: keep the one for the reported machine,
# archive any other (the PC moved bays).
mine = AssetRelationship.query.filter(
AssetRelationship.sourceassetid == pcasset.assetid,
AssetRelationship.relationshiptypeid == controls.relationshiptypeid,
AssetRelationship.label == MACHINE_LINK_ORIGIN,
).all()
link = None
for rel in mine:
if rel.targetassetid == machine.assetid:
rel.isactive = True
link = rel
else:
rel.isactive = False
if link is None:
link = AssetRelationship(
sourceassetid=pcasset.assetid,
targetassetid=machine.assetid,
relationshiptypeid=controls.relationshiptypeid,
label=MACHINE_LINK_ORIGIN,
isactive=True)
db.session.add(link)
# Any OTHER pc still collector-linked to this machine has been replaced.
superseded = AssetRelationship.query.filter(
AssetRelationship.targetassetid == machine.assetid,
AssetRelationship.relationshiptypeid == controls.relationshiptypeid,
AssetRelationship.label == MACHINE_LINK_ORIGIN,
AssetRelationship.sourceassetid != pcasset.assetid,
AssetRelationship.isactive.is_(True),
).all()
for rel in superseded:
rel.isactive = False
old = db.session.get(Asset, rel.sourceassetid)
oldname = old.assetnumber if old else str(rel.sourceassetid)
warnings.append(
'machine {} was taken over from {}; check that PC'.format(
machine.assetnumber, oldname))
self._alert_pc_superseded(oldname, comp, machine)
return [{'assetid': machine.assetid,
'machinenumber': machine.assetnumber,
'superseded': len(superseded)}]
def _alert_pc_superseded(self, oldname, newcomp, machine):
"""Tell a human a PC was replaced on a machine. Best effort, never raises.
Deliberately an ALERT and not a status change: the collector cannot tell
whether the old PC was shelved, sent for repair or re-imaged for another
bay, so it says what happened and lets a person decide. Uses the same
email + webhook path as the toner alerts, falling back to the site-wide
alert_recipients when nothing more specific is set.
NOT a shopfloor notification: that board is for operators (General,
Recertification, Recognition), and an IT asset message does not belong
in front of the floor.
"""
import logging as _logging
try:
from shopdb.api import send_email, send_webhook, Setting
newname = newcomp.hostname if newcomp else 'a new PC'
machinename = machine.assetnumber or machine.name
subject = 'PC replaced on machine {}: {} to {}'.format(
machinename, oldname, newname)
# /pcs/:id is keyed on computerid, not assetid - an assetid here
# opens someone else's PC or a 404.
base = (Setting.get('site_base_url') or '').rstrip('/')
link = '{}/pcs/{}'.format(base, newcomp.computerid) if base else ''
linkhtml = ('<p><a href="{0}">View {1}</a></p>'.format(link, newname)
if link else '')
html = (
'<p><strong>{0}</strong> is now reporting machine '
'<strong>{1}</strong>, which was previously driven by '
'<strong>{2}</strong>.</p>'
'<p>The old link has been archived. {2} has NOT had its status '
'changed - set it to Inventory, In Repair or Retired as '
'appropriate.</p>{3}'.format(newname, machinename, oldname,
linkhtml))
send_webhook(subject, subject)
raw = Setting.get('alert_recipients') or ''
recipients = [a.strip() for a in raw.replace(';', ',').split(',')
if a.strip()]
if recipients:
send_email(recipients, subject, html)
except Exception:
_logging.getLogger(__name__).exception(
'PC-superseded alert failed for machine %s',
getattr(machine, 'assetnumber', '?'))
def _sync_measuringtool_link(self, pcasset, pctype, hostname, warnings):
"""Auto-create + link the MeasuringTool a metrology PC drives.

196
tests/test_core/test_collector_contract.py
View File

@@ -145,7 +145,12 @@ def test_complete_asset_payload_maps_enrollment_data(client, db, collector_key,
with client.application.app_context():
comp = Computer.query.filter(Computer.hostname.ilike('WJSF1234')).first()
assert comp is not None
assert comp.asset.assetnumber == '0615' # machinenumber -> assetnumber
# The PC keeps its OWN assetnumber (hostname). machinenumber names the
# MACHINE, and machines are assets too (ADR-001), so assigning it here
# hit the unique index on assets.assetnumber and returned 500 to the bay
# on every retry. The machine number now builds a controls link instead,
# which is what the reported number was actually for.
assert comp.asset.assetnumber == 'WJSF1234'
assert comp.asset.serialnumber == 'SN-ENROLL'
assert comp.computertype.computertype == 'Shopfloor PC' # pctype mapped
assert comp.vendor.vendor == 'Dell' # created
@@ -528,3 +533,192 @@ def test_unknown_protocol_warns_and_does_not_create_one(client, db, collector_ke
assert any('TeamViewer' in w for w in body['warnings'])
assert AccessProtocol.query.filter(
AccessProtocol.name.ilike('TeamViewer')).first() is None
# =============================================================================
# PC -> machine link, and the PC-swap case
# =============================================================================
@pytest.fixture
def machine_3015(db):
"""A machine asset that already owns the number a PC will report."""
from shopdb.core.models import AssetType, Asset, RelationshipType
mt = AssetType(assettype='machine', pluginname='machines',
tablename='machines', description='Machines')
db.session.add(mt)
db.session.add(RelationshipType(relationshiptype='controls',
isdirectional=True))
db.session.flush()
asset = Asset(assetnumber='3015', name='Machine 3015',
assettypeid=mt.assettypeid, statusid=1)
db.session.add(asset)
db.session.commit()
return asset
def _links(machineassetid, activeonly=True):
from shopdb.core.models import AssetRelationship
q = AssetRelationship.query.filter_by(targetassetid=machineassetid)
if activeonly:
q = q.filter_by(isactive=True)
return q.all()
def test_pc_reporting_an_existing_machine_number_does_not_500(
client, db, collector_key, computer_assettype, machine_3015):
"""The machine owns assetnumber 3015. Naming the PC 3015 too hit the unique
index and returned 500 to the bay on every retry, forever."""
r = client.post('/api/collector/computers',
json={'hostname': 'FFBWTH63', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert r.status_code == 200, r.get_json()
def test_pc_gets_its_own_assetnumber_not_the_machines(
client, db, collector_key, computer_assettype, machine_3015):
from plugins.computers.models import Computer
client.post('/api/collector/computers',
json={'hostname': 'FFBWTH63', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
comp = Computer.query.filter(Computer.hostname.ilike('FFBWTH63')).first()
assert comp.asset.assetnumber == 'FFBWTH63'
def test_reported_machine_number_creates_the_link(
client, db, collector_key, computer_assettype, machine_3015):
client.post('/api/collector/computers',
json={'hostname': 'FFBWTH63', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert len(_links(machine_3015.assetid)) == 1
def test_replacing_a_pc_hands_the_machine_over(
client, db, collector_key, computer_assettype, machine_3015):
"""The point of the whole thing: the new PC takes the bay and the retired
one stops being shown as its controller."""
from shopdb.core.models import Asset
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
r = client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
active = _links(machine_3015.assetid)
assert len(active) == 1
owner = db.session.get(Asset, active[0].sourceassetid)
assert owner.assetnumber == 'NEWPC'
# and it says so, so a person can go and look at the old one
assert any('taken over' in w for w in r.get_json()['data']['warnings'])
def test_the_old_link_is_archived_not_deleted(
client, db, collector_key, computer_assettype, machine_3015):
"""History has to survive: 'which PC ran 3015 in June' stays answerable."""
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert len(_links(machine_3015.assetid, activeonly=False)) == 2
def test_the_old_pc_status_is_left_alone(
client, db, collector_key, computer_assettype, machine_3015):
"""The collector cannot tell if it was shelved, broken or re-imaged, so it
must not guess - and must not overwrite what a person set."""
from plugins.computers.models import Computer
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
old = Computer.query.filter(Computer.hostname.ilike('OLDPC')).first()
before = old.asset.statusid
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
db.session.refresh(old.asset)
assert old.asset.statusid == before
def test_unknown_machine_number_warns_and_invents_nothing(
client, db, collector_key, computer_assettype, machine_3015):
from shopdb.core.models import Asset
r = client.post('/api/collector/computers',
json={'hostname': 'FFBWTH63', 'machinenumber': '9911'},
headers={'X-API-Key': collector_key})
assert any('9911' in w for w in r.get_json()['data']['warnings'])
assert Asset.query.filter(Asset.assetnumber == '9911').first() is None
def test_the_placeholder_machine_number_links_nothing(
client, db, collector_key, computer_assettype, machine_3015):
"""9999 is the imaging-time placeholder, not a real bay."""
r = client.post('/api/collector/computers',
json={'hostname': 'FFBWTH63', 'machinenumber': '9999'},
headers={'X-API-Key': collector_key})
assert r.status_code == 200
assert _links(machine_3015.assetid) == []
def test_supersede_alerts_email_and_webhook(client, db, collector_key,
computer_assettype, machine_3015,
monkeypatch):
"""A person has to find out. Email goes to the site alert_recipients and the
webhook fires, the same path the toner alerts use."""
from shopdb.core.models import Setting
import shopdb.api as shopdbapi
# Patch on shopdb.api, which is where the plugin imports them from - the
# names are bound there at import time, so patching shopdb.utils.mailer
# would silently have no effect and the real (disabled) mailer would run.
sent = {}
monkeypatch.setattr(
shopdbapi, 'send_email',
lambda to, subj, html, **k: sent.update(email=(to, subj, html)))
monkeypatch.setattr(shopdbapi, 'send_webhook',
lambda subj, text, **k: sent.update(webhook=subj))
db.session.add(Setting(key='alert_recipients', value='it@example.com'))
db.session.add(Setting(key='site_base_url', value='https://shopdb/shopdb'))
db.session.commit()
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert 'webhook' in sent, 'webhook not fired on supersede'
assert 'email' in sent, 'no email sent on supersede'
assert sent['email'][0] == ['it@example.com']
assert '3015' in sent['email'][1] and 'OLDPC' in sent['email'][1]
# The link must be keyed on computerid: /pcs/:id resolves through
# GET /api/computers/<computerid>, so an assetid opens the wrong PC.
from plugins.computers.models import Computer
newpc = Computer.query.filter_by(hostname='NEWPC').first()
assert '/pcs/{}"'.format(newpc.computerid) in sent['email'][2]
def test_alert_failure_never_breaks_the_collector(client, db, collector_key,
computer_assettype,
machine_3015, monkeypatch):
"""A broken mail server must not stop a bay reporting its inventory."""
import shopdb.api as shopdbapi
def boom(*a, **k):
raise RuntimeError('smtp down')
monkeypatch.setattr(shopdbapi, 'send_email', boom)
monkeypatch.setattr(shopdbapi, 'send_webhook', boom)
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
r = client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert r.status_code == 200
assert len(_links(machine_3015.assetid)) == 1