32f4ccdd48
The sync route asked Dell about every pending service tag in one request. bulk_lookup sends 100 tags per HTTP call at a 15 second timeout, so a freshly-onboarded site was dozens of sequential calls inside a single web request. A gunicorn worker timeout or a proxy would plausibly kill it first, leaving the user with a failure after minutes and no way to tell what landed. F3 released the pooled connection before the vendor call, which stopped one sync from taking the app down, but did not make the request itself survivable. Extract the sync into a service taking a per-run tag limit. The route now caps at 200 tags - two Dell calls - and returns how many are still pending so the caller can run it again. A flask warranty sync-dell command does the same work from the command line for a large first-run backfill. The chunk is sampled at random rather than taken off a stable order. A serial Dell does not recognize never gains a warranty and so stays pending forever; under a stable order those dead tags would occupy every chunk and the sync would never reach the rest of the fleet. Nothing runs this on a schedule. The operation is used a few times a year, so a cron would idle-poll the warranty tables tens of thousands of times a year for nothing and log a configuration failure on every run at sites without Dell credentials. The two existing sync tests patched get_provider on the route module; the provider is now resolved in the service, so they patch it there.
165 lines
6.0 KiB
Python
165 lines
6.0 KiB
Python
"""The Dell routes must not hold a database connection across the vendor call.
|
|
|
|
A bulk sync asks Dell about 100 service tags per request and a fleet-sized run
|
|
is dozens of those, so a route that keeps its transaction open for the duration
|
|
pins a pooled connection for minutes. Under gevent workers that exhausts the
|
|
pool and takes down every other request, not just warranty.
|
|
|
|
These pin the boundary itself rather than the result: a fake provider inspects
|
|
the session at the moment it is called and records whether a transaction was
|
|
still open. Behaviour is covered elsewhere - what would silently regress here is
|
|
someone reading an ORM attribute after the commit, which re-checks a connection
|
|
straight back out and restores the original defect while every other test
|
|
continues to pass.
|
|
"""
|
|
|
|
from plugins.warranty.api import routes as warranty_routes
|
|
from plugins.warranty.services import dellsync as warranty_dellsync
|
|
from plugins.warranty.models import Warranty, WarrantyAsset
|
|
|
|
|
|
def _asset(db, assetnumber='PC-0001', serialnumber='ABC1234'):
|
|
from shopdb.core.models import AssetType, AssetStatus
|
|
from shopdb.core.models.asset import Asset
|
|
|
|
assettype = AssetType(assettype='computer')
|
|
status = AssetStatus(status='In Use')
|
|
db.session.add_all([assettype, status])
|
|
db.session.flush()
|
|
|
|
asset = Asset(
|
|
assetnumber=assetnumber,
|
|
name='Bench PC',
|
|
assettypeid=assettype.assettypeid,
|
|
statusid=status.statusid,
|
|
serialnumber=serialnumber,
|
|
)
|
|
db.session.add(asset)
|
|
db.session.commit()
|
|
return asset
|
|
|
|
|
|
class _ProbeProvider:
|
|
"""Stands in for DellProvider and records the session state when called.
|
|
|
|
The real provider blocks on HTTP here; what matters is only whether the
|
|
caller was still inside a transaction at that instant.
|
|
"""
|
|
|
|
def __init__(self, db, result):
|
|
self._db = db
|
|
self._result = result
|
|
self.in_transaction_at_call = None
|
|
self.called = False
|
|
|
|
def _observe(self):
|
|
self.called = True
|
|
# db.session is a scoped_session proxy and does not forward
|
|
# in_transaction; calling it returns the real Session for this scope.
|
|
self.in_transaction_at_call = self._db.session().in_transaction()
|
|
|
|
def lookup(self, servicetag, vendor=None):
|
|
self._observe()
|
|
return self._result
|
|
|
|
def bulk_lookup(self, servicetags):
|
|
self._observe()
|
|
return {tag: self._result for tag in servicetags}
|
|
|
|
|
|
def test_refresh_releases_the_connection_before_the_vendor_call(
|
|
client, db, auth_headers, monkeypatch):
|
|
warranty = Warranty(vendor='Dell', servicetag='ABC1234', provider='dell')
|
|
db.session.add(warranty)
|
|
db.session.commit()
|
|
warrantyid = warranty.warrantyid
|
|
|
|
probe = _ProbeProvider(db, {
|
|
'servicelevel': 'ProSupport',
|
|
'startdate': '2024-01-01',
|
|
'enddate': '2027-01-01',
|
|
})
|
|
monkeypatch.setattr(warranty_routes, 'get_provider', lambda name: probe)
|
|
|
|
response = client.post(f'/api/warranty/{warrantyid}/refresh', headers=auth_headers)
|
|
|
|
assert response.status_code == 200, response.get_json()
|
|
assert probe.called
|
|
assert probe.in_transaction_at_call is False, (
|
|
'refresh_warranty held a database connection across the vendor lookup'
|
|
)
|
|
# The write half still has to land, or releasing early would be a
|
|
# regression dressed up as a fix.
|
|
refreshed = db.session.get(Warranty, warrantyid)
|
|
assert refreshed.servicelevel == 'ProSupport'
|
|
assert refreshed.lastcheckeddate is not None
|
|
|
|
|
|
def test_sync_dell_releases_the_connection_before_the_bulk_call(
|
|
client, db, auth_headers, monkeypatch):
|
|
asset = _asset(db)
|
|
assetid = asset.assetid
|
|
|
|
probe = _ProbeProvider(db, {
|
|
'servicelevel': 'Basic Onsite',
|
|
'startdate': '2023-05-02',
|
|
'enddate': '2026-05-02',
|
|
})
|
|
monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: probe)
|
|
|
|
response = client.post('/api/warranty/sync/dell', headers=auth_headers)
|
|
|
|
assert response.status_code == 200, response.get_json()
|
|
assert probe.called
|
|
assert probe.in_transaction_at_call is False, (
|
|
'sync_dell held a database connection across the Dell bulk lookup'
|
|
)
|
|
body = response.get_json()['data']
|
|
assert body['created'] == 1
|
|
assert body['matched'] == 1
|
|
|
|
links = WarrantyAsset.query.filter_by(assetid=assetid).all()
|
|
assert len(links) == 1
|
|
created = db.session.get(Warranty, links[0].warrantyid)
|
|
assert created.provider == 'dell'
|
|
assert created.servicetag == 'ABC1234'
|
|
|
|
|
|
def test_sync_dell_updates_instead_of_duplicating_on_a_recheck(
|
|
client, db, auth_headers, monkeypatch):
|
|
"""Preloading the existing links must not lose the Dell-by-any-signal match.
|
|
|
|
The write loop now reads from one preloaded query instead of a query per
|
|
asset. A manually-added Dell warranty carries provider 'manual', so if the
|
|
preload missed it a re-check would create a second warranty for the asset
|
|
rather than updating the one already there.
|
|
"""
|
|
asset = _asset(db)
|
|
assetid = asset.assetid
|
|
|
|
existing = Warranty(vendor='Dell', servicetag='ABC1234', provider='manual')
|
|
existing.links.append(WarrantyAsset(assetid=assetid))
|
|
db.session.add(existing)
|
|
db.session.commit()
|
|
existingid = existing.warrantyid
|
|
|
|
probe = _ProbeProvider(db, {
|
|
'servicelevel': 'ProSupport Plus',
|
|
'startdate': '2024-02-02',
|
|
'enddate': '2028-02-02',
|
|
})
|
|
monkeypatch.setattr(warranty_dellsync, 'get_provider', lambda name: probe)
|
|
|
|
response = client.post('/api/warranty/sync/dell?all=true', headers=auth_headers)
|
|
|
|
assert response.status_code == 200, response.get_json()
|
|
body = response.get_json()['data']
|
|
assert body['created'] == 0
|
|
assert body['updated'] == 1
|
|
|
|
links = WarrantyAsset.query.filter_by(assetid=assetid).all()
|
|
assert len(links) == 1, 'a re-check duplicated the warranty instead of updating it'
|
|
updated = db.session.get(Warranty, existingid)
|
|
assert updated.provider == 'dell'
|
|
assert updated.servicelevel == 'ProSupport Plus'
|