feat: make the last sync only middleware async capable - #964
Open
blaipr wants to merge 32 commits into
Open
Conversation
The twilio SDK is here for a single call, messages.create, which is a form-encoded POST to the Messages endpoint with the account SID and token as basic auth. The backend now makes that POST with requests. Same fields, To, From and Body, and the same handling of a partial failure: the first exception is kept and raised after every recipient has been tried, rather than at the point it happened. Two things the SDK was hiding. There is no client to construct, so the branch that caught a construction failure and then used the unbound connection anyway is gone. And the request carries AWX_NOTIFICATION_REQUEST_TIMEOUT, so a stalled endpoint can no longer hold a dispatcher worker. twilio leaves the tree and takes aiohttp-retry with it. aiohttp stays, since requirements.in and kubernetes both ask for it, and its via list is updated to say so. test_twilio.py mocked the SDK client and now mocks the POST, keeping the three cases it already covered.
Every process that touches the credential models loads every credential plugin, because awx/main/models/credential/__init__.py walks the awx.credential_plugins entry points and calls load() on each one at import time. So boto3, botocore, azure-identity and azure-keyvault-secrets are imported by the web workers, the dispatcher and every management command, whether or not an AWS or Azure credential exists anywhere. Both plugins now import their SDK inside the lookup function, where it is used. Measured here on the two plugin modules alone, importing them drops from 248 ms to 72 ms and none of the four SDK modules ends up in sys.modules. azure_kv needed one more change to get there: AUTHORITY_HOSTS was built from AzureAuthorityHosts constants at module scope, which is what forced azure.identity to load just to define the cloud_name choices. Those constants are string literals, and this module already spelled the German one out, so all four are now literals with a test asserting they still match what the SDK defines. test_azure_backend_authority patched the SDK names on azure_kv, which no longer has them, so it patches azure.identity and azure.keyvault.secrets instead.
certifi and ansible-runner installed from @devel, so two of the three git requirements were whatever those branches happened to point at when an image was built. Two builds of the same tag could differ, and a bad upstream commit would arrive without anyone choosing it. Both are pinned to the commit their branch points at today, which is the form python3-saml already uses. ansible-runner lands on the commit that removed the python-daemon dependency, so the pin captures that rather than leaving it to timing. Renewing a pin is now a deliberate change with a diff, which is the point.
npm runs a package's install scripts by default, which is how the Axios
class of supply chain attack executes: a compromised release does its work
during npm ci, before anything is built or reviewed.
Nothing here needs them. The lockfile has two packages with install scripts:
fsevents, which is optional and macOS only, and unrs-resolver, whose native
binding is also published as a platform optionalDependency, so npm installs
it without running anything.
All three install sites are covered: the Makefile target the image build
reaches through ui-release, the CI job, and awx/ui/Dockerfile.
Checked on node 24: npm ci --ignore-scripts installs cleanly and reports no
vulnerabilities, the linux-x64 resolver bindings are present, eslint runs
without import resolution errors, and vite build produces the usual tree,
index.html plus static/{css,js,media}.
Nothing in CI looks at whether a shipped dependency has a known vulnerability. Both halves are added advisory, the way the type check already is, so they report a backlog without blocking anyone. Dropping advisory: true and continue-on-error makes them gates once a clean run is the normal state. The Python half runs inside ascender_devel and audits the environment the image installed, rather than resolving requirements.txt again. Resolving it outside that image fails: pip rejects the pinned combination of channels, django-auth-ldap, django-cors-headers and django-guid against Django 6.1.1 even though uv resolved it and it installs. Auditing what actually ships avoids the question entirely. The UI half has no image to reuse, so it is a small job of its own, with --omit=dev because a vulnerability in a build-time package is not shipped to anyone and the noise would bury what is.
An Insights project cannot be created: 'insights' has not been in the scm_type choices since the fork dropped it, and the platform refuses the value. What stayed behind is the machinery that served it. Gone with it: - the scm_type branches in Project.clean_scm_url and clean_credential, including the rule that an Insights project needs an Insights credential - 'insights' in update_scm_url, its http/https scheme entry, and its exemption from the username check - the scm_type branch and the insights_url extra var in the project update task - the "Project update for Insights" play, its update_insights tags, and the two modules it called, playbooks/library/insights.py and playbooks/action_plugins/insights.py - INSIGHTS_URL_BASE, which only those paths read Two settings that look like they belong here do not: INSIGHTS_AGENT_MIME and INSIGHTS_CERT_PATH are read by awx/main/analytics/core.py and the analytics API view, so they belong to automation analytics and stay. Tests: the unit test for the Insights credential rule and the insights case in update_scm_url go with the code they covered. Two fixtures go too, both unused: insights_project, and insights_inventory, which set scm_type on an Inventory, a model that has no such field. Nothing here touches the 'insights' credential kind or the commented out inventory source, which are separate removals.
Nothing can have this kind. There is no managed Insights credential type, and a custom credential type is restricted to 'cloud' or 'net' by the serializer, so the choice has been unreachable since the Insights project type stopped existing. Removed with it: the two inventory source validation messages that listed insights among the disallowed kinds, and the entry in the UI's unacceptableCredentialTypes list, which named a credential namespace that cannot exist. Migration 0215 carries the choices change. PostgreSQL does not enforce choices, so it emits no meaningful schema change and a row carrying kind='insights' in a database migrated from AWX keeps loading exactly as it did. What the migration is for is Django's own state: without it the autodetector sees the model and the migrations disagree, makemigrations offers a spurious migration on every run, and the next real migration anybody generates silently carries this change inside it. Deleting such rows is a decision for whoever runs that install, not for a migration to make on their behalf: the kind was already unusable, since the project type that consumed it is gone.
Three settings are left from the Insights inventory source: the instance id var, the empty group exclusion, and the system id file path, plus two more commented out beside them. Nothing in the tree reads any of them, in Python or in the playbooks and templates. The three that remain are not this feature. INSIGHTS_TRACKING_STATE is the automation analytics toggle, and INSIGHTS_AGENT_MIME and INSIGHTS_CERT_PATH are read by the analytics upload, so they belong with the subscription work rather than here. The commented out inventory source choice and plugin injector stay as they are. Commented entries are how this fork records what it has dropped, and the collection sweep reads them to tell a deliberate removal from an oversight, so deleting them would lose that signal.
make ui-api-types did not reproduce the file it is documented to produce. Regenerating it changed 73,011 lines, because the committed copy carries a header banner and prettier formatting that the target never applied. The artifact and its generator had quietly diverged, which is the one thing a generated file cannot afford. generate-api-types now does what produced the committed file: openapi-typescript, then prettier, then the banner, which lives in scripts/api-types-header.txt rather than being pasted by hand. Regeneration is byte identical afterwards. With that true, CI can check it: ui-api-types-drift regenerates the types and fails if the committed copy differs, so a serializer change that nobody followed with make ui-api-types stops being invisible. The types are regenerated here, which drops the insights credential kind removed earlier in this series: the first real drift the check would have caught. awx/ui/.schema.json stops being tracked. It is the intermediate the generator feeds on and deletes, 2 MB of it, and it arrived in the repository by accident; .gitignore keeps it out.
There is no SecurityMiddleware and no SECURE_* settings, so nosniff, the referrer policy, the opener policy and frame options all depend on whichever proxy is in front. The development compose nginx sets two of them; what a given deployment sets is anyone's guess. The application sets them for itself now: SecurityMiddleware first in the list so its headers reach responses that later middleware short circuits, XFrameOptionsMiddleware last, and four settings to go with them. A deployment that also sets them at the proxy gets the same values, not a conflict. HSTS is deliberately left out. Django only emits it on a request it believes is HTTPS, and behind a TLS terminating proxy it sees HTTP unless SECURE_PROXY_SSL_HEADER is configured, so setting it here would be silently inert on exactly the deployments that need it. The proxy sets it. The test drives the two middlewares directly. The functional fixtures call views through APIRequestFactory, which never runs middleware, and the test database is built with --nomigrations, so a real request through the stack trips MigrationRanCheckMiddleware querying a table that does not exist.
The end-to-end suite drives the UI in a real browser and never looks at whether anyone using a screen reader could follow it. @axe-core/playwright scans the rendered page against WCAG 2 A and AA, and the spec runs with the rest of the suite, so no workflow change is needed. It compares what a screen violates against a recorded baseline and fails only on something new, because a zero violation gate on an application that does not pass yet would fail on arrival and be switched off within a week. No baseline file ships: all three screens scanned, login, jobs and templates, are clean. They are clean because the scan found something real and it is fixed here rather than recorded. index.html carried no title, and Login.tsx only sets one once its branding request resolves, so until then the document had no title at all: axe reports document-title, a screen reader announces nothing useful, and the browser tab shows the URL. index.html now ships a static title, which the branded one still overrides. Verified against a development stack built from this branch: the violation reproduced before the fix and all three screens pass after it.
The type checker runs advisory over the whole tree, which is right while 2,896 diagnostics are being worked through, but it means a package that reports nothing today can start reporting tomorrow and nobody notices. typecheck-strict is the ratchet: the same checker, failing, over the twelve packages that already report nothing. The advisory run still covers everything else, and a package moves up here the moment it comes clean. Where the diagnostics actually are, for whoever works the backlog next: tests 1,107, models 603, api/views 144, api/serializers 112, sso tests 92, credential_plugins 89, tasks 79.
Nothing says what is inside a release image or where it came from. A user
who pulls ghcr.io/ctrliq/ascender:25.6.2 has the registry's word for it and
nothing else.
stage.yml, after the multi-arch manifest exists: read its digest, generate
an SPDX SBOM with syft, and attest both the SBOM and the build provenance
to the registry. promote.yml signs the digest the release points at with
cosign.
Everything is keyed on the digest rather than the tag. A tag can be moved,
including by promote.yml when it points :latest at a new release, and a
signature over a tag would then describe something else.
No keys to hold. attest-* and cosign sign through Sigstore with the
workflow's own OIDC identity, which is why both jobs gain id-token: write.
Verification needs no secret either:
cosign verify ghcr.io/ctrliq/ascender@<digest> \
--certificate-identity-regexp '^https://github.com/ctrliq/ascender/' \
--certificate-oidc-issuer https://token.actions.githubusercontent.com
Every action is pinned to a commit with its version in the comment, as the
rest of the workflows are.
There is no React.lazy anywhere, so every screen, and everything it imports, shipped in the first bundle: 3.21 MB of JavaScript before the login form can render, including ace-builds for an editor most sessions never open. routeConfig declares its twenty-five screens with React.lazy, App does the same for the two it routes to itself, and both Routes blocks render inside a Suspense boundary with the existing ContentLoading as the fallback. Login stays eager, since an unauthenticated visitor needs it immediately. Measured with vite build, before and after: chunks 16 -> 130 first bundle 3.21 MB -> 381 KB, 88% smaller total JS 4.46 MB -> 4.52 MB The total grows slightly, which is the cost of chunking, and the first paint stops paying for screens nobody opened. ace-builds is now its own 684 KB chunk that loads when an editor appears. Both UI suites pass unchanged: 177 files and 1,054 tests general, 374 files and 1,937 tests screens. eslint, prettier and tsc clean.
The setting notifications and job output links are built from still carried the Tower name, in the API, in the UI settings screen and in 56 places in the tree. Renamed, with the stored value carried across by conf migration 0011, which uses the same rename_setting helper the three earlier setting renames used. A deployment that sets the old name in /etc/tower/conf.d keeps working for a release through a fallback in production.py; the installers set it through the API, which the migration covers. Also renamed, in the same sweep: - the TOWER_SECRET_KEY environment variable that regenerate_secret_key reads, now ASCENDER_SECRET_KEY, with the old name still honoured - the Ascender credential type injects ASCENDER_HOST, ASCENDER_USERNAME, ASCENDER_PASSWORD, ASCENDER_VERIFY_SSL and ASCENDER_OAUTH_TOKEN beside the TOWER_ and CONTROLLER_ names it already sets, so ascender-kit and the collection have something to move to Left alone deliberately: LOG_AGGREGATOR_TOWER_UUID and the tower_uuid field in the log payload. That field is a wire format that Ledger and any other aggregator read, so renaming it belongs with the logging work rather than here. Verified against a running deployment rather than in theory: seeded TOWER_URL_BASE in the database, ran awx-manage migrate conf, and the value arrived under the new key with the old row gone; the settings API then serves ASCENDER_URL_BASE and no longer offers the old name.
Every response embeds a copy of each related object under summary_fields, so a page of fifty job templates carries fifty copies of the same organization, inventory, project and capability map. Nothing can ask for less, because summary_fields is built unconditionally in the base serializer. A summary_fields query parameter now says what is wanted: absent means every field, exactly as before, so nothing changes for an existing caller. "none", "false" or "0" asks for an empty object, and a comma separated list asks for those keys only. The parameter joins the reserved names, since without that the filter backend reads it as a field to filter on and answers 400. Measured on a running deployment, a fifty item job template list: default 178,835 bytes 0.521 s summary_fields=none 133,158 bytes 0.415 s summary_fields=organization,user_cap.. 140,731 bytes 26% smaller and a fifth quicker, on test data whose summary fields are nearly empty. A real deployment carries recent_jobs, credentials, the last job and the user who made it, so the saving there is larger. Asking for none skips building the fields at all, which is where the time goes; asking for a subset still builds them and then filters, so it saves bytes rather than queries. Narrowing that is worth doing when something needs it.
set_statement_timeout reads PostgreSQL's statement_timeout off uwsgi's harakiri value, so a query is cancelled before the worker is killed. Off uwsgi it falls back to DATABASE_STATEMENT_TIMEOUT, which defaults to None, so no timeout is applied at all. That is fine while uwsgi is what serves. It stops being fine the moment anything else does, which is the first thing the ASGI move would change: daphne or uvicorn would serve HTTP with no statement timeout anywhere, and nothing would say so. A process that announces itself with AWX_WEB_PROCESS now gets DATABASE_STATEMENT_TIMEOUT, or 110 seconds when that is unset, which is what the uwsgi path works out to today: harakiri of 115 less a five second margin. The web supervisor programs, uwsgi and daphne both, set the variable. Task workers, management commands and migrations are untouched: they carry no marker, so they still get no timeout, because a long query there is the job rather than a symptom. Nothing changes for a uwsgi deployment: harakiri is still read first and still wins. This is step one of the path in the ASGI thread, and it is worth having whether or not the rest of it happens.
CodeEditor is the only non-test file that imports ace, and ace is the largest thing the UI ships: its chunk was 699.78 kB, 193.77 kB gzipped, the biggest in the application, because ace-builds arrives whole and its modes and themes are separate files loaded for their side effects. CodeMirror 6 ships only the pieces that are imported. The same component on the same props is now 433.07 kB, 141.01 kB gzipped, and the whole bundle drops from 4,735,923 to 4,468,204 bytes. The component's interface does not change, so no caller does: the same props, the same four modes, the same auto height between minRows and maxRows, the same Enter to edit and Escape to leave, the same debounce, the same Ctrl-F search. Two things behave better rather than the same. The editor renders only the lines in view, where ace built a line of DOM per line of content, which is what MAX_ROWS was capping. And a read-only editor is now a tab stop, because its scroll area was otherwise unreachable from the keyboard, which axe reports as scrollable-region-focusable. The colours are ace's twilight palette to the hex, but they are now CSS variables with those values as fallbacks. The four themes set the variables on .cm-editor instead of overriding rules with !important, which is what they were doing to reach ace's classes. The tests gain from it. The editor holds its document in the DOM, one element per line, so what it shows and what is typed into it are both observable: three test files stop mocking the editor away, and the assertions dropped when the suite moved to RTL, formatted JSON, yaml to json conversion, the empty-value defaults and the edit-then-submit path, come back.
The accessibility spec added with the end-to-end suite went red on the job and template lists, reporting button-name and link-name on the page header, and color-contrast on the sorted column header. The header one is real. The pending approvals badge is a PatternFly NotificationBadge, which renders a button, wrapped in a Link. The button carries an aria-hidden bell icon and, at zero pending, no text at all, so neither it nor the link around it has a name. The button also has no handler: it was a second tab stop that did nothing, since the link around it is what navigates. So the badge is the link now, through PatternFly's component prop, with an aria-label on it. One element, one tab stop, one name, and it is still an anchor, so it still opens in a new tab. The contrast one is recorded rather than fixed. The sorted column's header is drawn in the brand green, #0e8c5d, which is 4.25:1 on the table background where AA asks for 4.5. It comes from the brand colour itself, so moving it is a palette decision across four themes rather than a patch: the same green is 4.26:1 on the light themes' white, and the lighter #12a66f that fixes the dark ones is 3.13:1 there. That wants a colour picked on purpose. The baseline file the spec has always looked for now exists and carries it, with the reasoning in the spec. Checked in a browser against the development stack: the jobs list scans clean, and the templates list reports only the recorded contrast. All three accessibility specs pass.
CONN_MAX_AGE is unset, which is Django's default of 0: every request opens a PostgreSQL connection and closes it when the response is sent. The next request opens another. Nothing pools, so that is the cost of every API call before it has run a single query. Measured against the development stack, fifty requests to /api/v2/ping/ opened fifty one database sessions. Establishing one costs 5.2 ms on a container to container connection, and 5.7 ms once the application_name and statement_timeout options are in it, against 0.29 ms for a query on a connection that already exists. A process that serves HTTP now keeps its connection for sixty seconds. The same fifty requests open five sessions, one per uwsgi worker, which is a 90% cut in connection churn for no change anyone can see. Sixty seconds is deliberately short. An idle worker holds its connection until its next request notices the age, so what is held at rest is one per web worker, and a short life keeps a connection from outliving a network path that has quietly gone away. Task workers, management commands and migrations are left alone. They hold one connection for the life of a long process, or want it closed the moment the command ends, and an age limit helps neither. DATABASE_CONN_MAX_AGE overrides all of it, including for a task process, so a deployment can turn reuse off with 0 or hold connections for longer. Web is detected the way the statement timeout detects it: uwsgi answers for every deployment today, and AWX_WEB_PROCESS answers for anything that is not uwsgi, which is what will matter if the server ever becomes daphne.
SettingsCacheMiddleware clears this process's in-memory settings cache at the start of every request. The cache therefore never survives one, so every setting read during the request goes to the shared cache instead. It is there for a reason: a script can POST to /api/v2/settings/all/ and immediately GET it back, and expect the new value even when the two requests land on different workers. But the price is paid by every request to cover a setting write that almost never happens. Measured against the development stack, 200 requests each way: /api/v2/ping/ 7.11 cache round trips per request, now 2.10 /api/v2/me/ 12.45 cache round trips per request, now 5.38 Every setting change now bumps one shared counter, and the middleware reads that one key and drops the in-memory cache only when it has moved since this process last looked. Read-your-writes is unchanged, and that is checked rather than argued: with five uwsgi workers warmed, a PATCH moves the counter from absent to 1, and twenty immediate reads across those workers all return the new value. A cache that cannot be read drops the in-memory copy and logs it, which is what the old code did on every request anyway, so an unreachable cache degrades to today's behaviour rather than to stale settings.
cigamit said it on the roadmap thread: the license code was left in because it was not yet clear whether we would use it internally for a supported version, we do not, so it can come out. This is the first piece of it. Two endpoints existed to talk to Red Hat. /api/v2/config/subscriptions/ took a Red Hat or Satellite username and password, read the server hostname out of /etc/rhsm/rhsm.conf, and asked subscription.rhsm.redhat.com or a Satellite for that account's entitlements. /api/v2/config/attach/ then picked a pool out of the answer and made it the installed license. The UI offered it as one of two ways to activate a subscription, storing the credentials for later use. None of it means anything for Ascender. It reaches a service we have no account with, for entitlements that do not describe this product, and it kept a customer's Red Hat password in the settings table to do it. Out of the platform: both views and their routes, Licenser.validate_rh and the rhsm and Satellite lookups behind it, generate_license_options_from entitlements, and rhsm_config, which read /etc/rhsm/rhsm.conf. Out of the settings registry: SUBSCRIPTIONS_USERNAME and SUBSCRIPTIONS_PASSWORD, which nothing reads now. A row either has in the Setting table is inert, rather than deleted, since removing a customer's stored data is their call. Out of the UI: the username and password half of the wizard's first step, the toggle that chose between the two, SubscriptionModal, which listed what came back, and the two api client methods. What stays is the manifest. Uploading a Red Hat subscription manifest still works exactly as it did, which is the path an Ascender install actually uses, and validate_entitlement_manifest and license_from_manifest are untouched.
The only retention story the largest table in the database had was cleanup_jobs, which drops an event partition once the last job pointing at it is deleted. That ties how long output is kept to how long jobs are kept, and they are not the same question: a year of job history is cheap, a year of every line those jobs printed is not. cleanup_job_events answers the second question on its own. It drops the event partitions whose hour has fallen out of the window and leaves the jobs alone, so the history stays and the output behind it does not. It is off unless asked. JOB_EVENT_RETENTION_DAYS is 0, which means no window, and the command refuses to run without one rather than guessing. A deployment that does nothing keeps exactly what it keeps today. A partition is only dropped once the hour it covers has ended before the cutoff, so the hour the cutoff falls inside survives. Anything whose name does not read as an hour is left alone: the _unpartitioned tables that predate partitioning, and anything a person has attached by hand. Checked against the development stack: with 152 jobs and 6,313 job events across 37 partitions, --days 1 dropped 78 partitions and took the events with them, and all 152 jobs were still there afterwards. The job list, a job's detail, its job_events and its stdout all still answer 200.
Eight access classes are read only: job events, ad hoc command events, project and inventory update events, system job events, job host summaries, the activity stream and workflow approval votes. Each said so by repeating the same three methods, which is fifty four lines of the same thing and, more to the point, makes "nobody can write this" something you learn by reading three method bodies rather than something the class states. ReadOnlyAccess says it once. What each class carries afterwards is only what differs about it: its model, and which of its rows a user is allowed to see. It is a mixin listed before BaseAccess, which is the shape the file already uses for NotificationAttachMixin and UnifiedCredentialsMixin, and which matters for a second reason: access_registry is built by walking BaseAccess.__subclasses__(), so a class that stopped being a direct subclass would silently drop out of it. UnpartitionedJobEventAccess is registered by hand two lines further down for exactly that reason. ReceptorAddressAccess looks like one of the eight and is not. Its three methods carry check_superuser, so a superuser can manage receptor addresses and only everyone else is refused. Moving it would have taken that away quietly, which is the whole hazard of this file. Nothing changes, and that is checked rather than asserted. Every access class was asked can_add, can_change and can_delete as a plain user and as a superuser, before and after: 48 classes, 288 answers, zero differences. The access registry maps the same 49 models to the same 49 classes.
awx/api/views/__init__.py is 4,780 lines and 303 view classes in no order, which is the file the roadmap asks to split by resource. The package already has per resource modules, inventory, organization, labels and the rest, so this is the pattern rather than a new one: the 55 workflow classes move into awx/api/views/workflow.py and the url modules import them from there. Templates, their nodes, the jobs they run and the approvals a workflow waits on, all in one file instead of scattered through four thousand lines. Nothing changed on the way across. The five shared bases they are built on, JobTemplateSurveySpec, JobTemplateLabelList, JobLabelList, LaunchConfigCredentialsBase and EnforceParentRelationshipMixin, stay where they are and are imported. _approval_vote_comment goes with the views that call it, and the module makes the same lowercase_with_underscore view functions the package makes for its own classes, so nothing is lost. Five imports in __init__.py became dead when the classes left, and are gone. That the routing is untouched is checked rather than claimed: every URL the resolver knows was mapped to the view class that serves it, before and after. 413 routes, zero differences, 53 of them served by a workflow view.
index.html is built as a Django template: it opens with a load static, and carries the CSP nonce Django renders per request in three places. That is what ties the file to Django rather than to a static file server, and it is the reason the build layout is a contract with staticfiles. All three nonce uses turn out to be unnecessary. window.NONCE_ID is set by an inline script and read by nothing, anywhere. It is what is left of __webpack_nonce__, which only webpack understood, and webpack went with the Vite move. That inline script is also the only thing that needed script-src to name a nonce at all. The sc-nonce meta tag is for the style tags styled-components injects, and the policy already allows those: style-src is 'self' 'unsafe-inline'. The one inline style tag, the app height rule, is covered by the same thing. The favicon went through a static tag so the build could reach it through staticfiles, which does not hash names here, so the plain path the source file already carries works. So none of them are needed, and with them gone the built file has no Django template syntax in it at all: the same bytes a static file server would hand out. The vite plugin that used to inject the template markup now injects only the policy and that one style rule, and is named for what it does. Checked in a browser against the development stack, on a page full of styled components: 34 style rules injected and applied, the themed background among them, zero content security policy violations, no sc-nonce meta, NONCE_ID undefined, and no console errors. The end to end suite passes all 14, and the UI suite all 2,989.
The roadmap item reads "AWX extracted its dispatcher into dispatcherd: adopt it, or own our frozen copy deliberately". Nothing in the tree says which, or even that there is an upstream to have an opinion about, so a reader of awx/main/dispatch has no way to know any of this. This writes it down: where the code came from, what the package it was extracted into looks like now, what is here instead and why, and what adopting would actually involve. The facts, checked rather than remembered. These modules are AWX's dispatcher as of the 24.0.0 sync in March 2024, the last this fork took. dispatcherd's first release is March 2025, so the extraction happened after that sync and the upstream of this code no longer lives in AWX. The package is now around 5,800 lines across 26 modules against the 1,800 here, and the difference is nearly all generalisation: brokers behind an interface with a socket broker beside PostgreSQL, producers as separate objects, its own config layer, and a testing package with in memory and error only brokers. No behaviour changes. It is a module docstring.
The UI fetches through useRequest, a hand-rolled hook that holds a result, an error and a loading flag. It works, and it has no cache: two components that want the same thing make the same call twice, and a screen reopened refetches what it read a second ago. This adds TanStack Query and moves the first two screens onto it, the ones that show the closest thing to a duplicate in the tree. HostFacts and InventoryHostFacts both read the same endpoint for the same host with nearly identical code, and now share a useHostFacts hook, one request and one cache entry. The point of the change is checked rather than described: two components asking for the same host's facts make one request, two different hosts make two, and a failure surfaces rather than hanging. Reads stay fresh for thirty seconds, which is short enough that a screen reopened after a change shows it and long enough that components mounting together share a request. Refetching on window focus is off, because these screens are often left open on a wall display and that setting turns one into a request storm. The test harness builds a client per render with retries off and no cache between tests, so nothing a test caches is ever seen by the next. The other 219 files still use useRequest, and this does not change them. It is the pattern, working, on the place with the clearest duplication.
The roadmap item says flatten UnifiedJob only if a feature needs it, and nothing in the tree says what the current shape costs, so the condition has never been checkable. Measured, on the development stack. A job is two rows, one in main_unifiedjob and one in its own table, and reading a mixed page costs one query for the base rows and one more per job type on the page. Fifty rows across five types is six queries. A hundred rows across the same five types is still six: the count is bounded by how many types are on the page, never by how many rows. Five queries look like an N+1 on a cold process and are not. They are django-polymorphic asking for each content type through ContentType.objects.get_for_id, which Django caches for the life of the process. The first evaluation in a worker is twelve queries, the second and every one after it is six. That bound is the reason flattening has not been worth doing, so this pins it rather than describing it. Five tests: the page costs one query per type, a page of thirty costs what a page of fifteen costs, a page of one type costs two, every row comes back as its real class, and the base rows alone are one query, which is the difference the shape actually buys. An N+1 introduced into this path now fails the suite rather than somebody's instance. The model says the same thing in a docstring, for whoever weighs the item next.
Six styled-components across five files in components/DetailList, which is the family every detail screen in the application is built from, so it is the place to show the exit rather than a corner nobody looks at. None of them needed styled-components. Every rule was static or a boolean variant: fullWidth set grid-column, isEncrypted and isNotConfigured both set the same muted colour, stacked chose between two column layouts. Those are modifier classes, so the styling is a css file now and the components add a class or do not. Nothing moves on screen, and that is checked rather than reviewed. Two detail pages were loaded in a browser before and after, and every computed property of the list, the name, the value and the row compared: 40 properties, zero differences. The grid is still three columns at 1400px with a 32px gap, the name is still 12px uppercase with 0.03em tracking, the row still has its 0.875rem padding. One fix carried along: useHostFacts.test.tsx imported ResponseOf from a path that does not exist, which vitest resolved and tsc did not. It was added in the TanStack change and never type checked, since that branch only ran eslint and the suite after the test file appeared. The other 145 files still use styled-components. This is the pattern, on the component family with the widest reach.
The roadmap asks for one form family ported off formik, to prove an exit exists before an unmaintained 2.4.9 becomes a problem. Trying it turned up why that is not the right first move. TeamForm is the smallest self-contained form in the tree, three fields and a lookup, and it cannot be ported alone: FormField, FormActionGroup, FormSubmitError and OrganizationLookup all bind to formik themselves, so a port needs a parallel field layer before it can start. Worse, the coupling is not behind that layer. Counting the imports: 129 files import formik directly, useField appears in 280 places, Formik in 152 and useFormikContext in 103. Of the 129, only 49 are in src/components. The other 80 are screens reaching for formik themselves, and every one of them is bound to it whatever its form component does. So the exit is gated on the field layer, not on any one family, and the first useful move is to stop the number growing. This is that ratchet: a screen that goes through FormField, CheckboxField, VariablesField or a Lookup does not move it, one that imports formik itself does and fails here, with a message saying which to use instead. Checked by breaking it on purpose: adding a formik import to one screen gives "81 screens import formik directly, up from 80" and a failure. Nothing is asserted about formik being broken today. It is not: 2.4.9 is the current release, its peer range is react >=16.8.0, which covers the 19.2.7 here, and 2,996 tests pass under a harness that fails on a console error.
The roadmap says async views wait on numbers, and gives two reasons: ATOMIC_REQUESTS is on, and every middleware is sync. The second one is not true, and has not been for a while. Asking each of the twenty middleware what it can handle: nineteen are already async capable, because MiddlewareMixin has been for several Django releases and most of this chain is built on it. Exactly one was not, and it is ours: LogTracebackMiddleware, in the vendored dab subset. One is enough to cost everything. Django wraps the whole chain below a sync only middleware, the view included, in sync_to_async, so a single class missing an attribute buys a thread hop per request for the other nineteen. This one is third from the top. It holds nothing thread local. It writes the request into a class dict keyed by a fresh uuid and pops it in a finally, so the SIGABRT handler can say where an in flight request was. That is the same under either model, so it now says sync_capable and async_capable and has an __acall__, the pattern the middleware in awx/main already uses. The chain is twenty async capable and zero sync only afterwards, which leaves ATOMIC_REQUESTS as the one reason left in that bullet. Pinned rather than described: a test asks every middleware in the configured chain the same question, so a new sync only one is a decision somebody takes rather than an attribute left off a class, and a test that it still handles a sync request, since uwsgi is what serves today.
blaipr
force-pushed
the
feat/async-capable-middleware
branch
from
September 13, 2026 08:58
d39e20a to
f1f85c8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sits on top of the stack ending at #963. From the roadmap list in To Do: "Async views only once that move gives numbers:
ATOMIC_REQUESTSis on, every middleware sync."Half of that bullet is wrong
I asked each of the twenty middleware what it can handle:
MiddlewareMixinhas been async capable for several Django releases, and most of this chain is built on it. The one exception is ours:LogTracebackMiddleware, in the vendored dab subset.One is enough to cost everything
Django wraps the whole chain below a sync only middleware, the view included, in
sync_to_async. So a single class missing an attribute buys a thread hop per request for the other nineteen, and this one is third from the top.Why it can simply say so
It holds nothing thread local. It writes the request into a class dict keyed by a fresh uuid and pops it in a
finally, so the SIGABRT handler can report where an in-flight request was. That is the same work under either model, so it now declaressync_capableandasync_capableand has an__acall__, the patternawx/main/middleware.pyalready uses forRequestContextMiddleware.Afterwards: 20 async capable, 0 sync only, which leaves
ATOMIC_REQUESTSas the one reason left in that bullet.Pinned, not just described
awx/main/tests/unit/test_middleware_is_async_capable.pyasks every middleware in the configured chain the same question, so a new sync only one becomes a decision somebody takes rather than an attribute left off a class. It also asserts each one still handles a sync request, since uwsgi is what serves today and async capable must not quietly mean async only.Plus two behaviour tests on the changed class: it adapts to whichever kind of
get_responseit is handed, and a request in flight is visible intransactionswhile the response is being produced and gone afterwards.Checks
awx/main/tests/unit: 1,374 passed, 1 skipped./api/v2/ping/,/api/v2/me/,/api/v2/job_templates/and/api/v2/settings/ui/all 200, and the SIGABRT handler is still installed.