Four calls reached a base method with an argument list it does not accept,
so they raised TypeError about the argument count.
Three of them would have failed either way, since the base only raises
UnsupportedFunctionality. What changes there is that the failure becomes
the intended, catchable one rather than an argument count error the caller
cannot interpret. get_diagnostic_data grew an autosuffix argument
everywhere except the ipmi generic handler, which is the handler used for
unrecognized hardware. The redfish generic handler already had it. The two
storage super() calls dropped the cfgspec they were given.
The fourth is a real fallback rather than a message: the XCC user_delete
dropped the fishclient it receives from redfish/command.py, so deleting a
uid the XCC does not list raised TypeError instead of attempting the
generic Redfish delete.
ConfluentTargetNotFound takes the node as its first argument, and every
other caller passes it. The two inventory plugins constructed it with no
arguments at all, so asking for a component that is not in the inventory
map raised TypeError instead of returning the 404 the path was written to
return.
Both now follow the pattern used a few lines further down for volumes and
name the component that was not found.
The retry around the firmware progress poll named socket.socket, which is not
an exception class, so the moment the request it guards actually failed Python
raised "catching classes that do not inherit from BaseException is not
allowed" in place of the error.
socket.error is OSError, which is what a failed poll raises and what the retry
below was written for.
The loop resolver takes only host and port positionally, so these three calls
raised "BaseEventLoop.getaddrinfo() takes 3 positional arguments but 5 were
given" every time they ran.
get_ipaddr and _find_service have no handler above them, so link local XCC
discovery and a targeted SSDP search both died outright. The snoop copy sits
under an except Exception, which swallowed it and left the MGTIFACE reply
unanswered instead.
get_sensor_names and get_sensor_descriptions reach get_psu_count for any
sensor whose table entry carries elementsfun, and get_psu_count is a
coroutine. As plain generators they could not await it, so range() was handed
the coroutine object and enumeration died with "'coroutine' object cannot be
interpreted as an integer".
Every DW612S has such entries, so nodesensors returned nothing for the
enclosure. get_sensor_descriptions was doubly broken: the Lenovo handler
already iterated it with async for, which a plain generator cannot satisfy.
Verified against a DW612S SMM (FPC variant 38). Before, descriptions raised at
the async for and readings raised partway through enumeration; after, both
return all 34 sensors, 19 of which are the PSU entries that never enumerated.
RUF006 catches a create_task whose result is discarded. The loop holds only a
weak reference, so such a task can be collected while still pending and the
work disappears without a trace.
Selected last, once the three existing offenders are gone, so the tree stays
clean under it from this commit on.
run_handler scheduled the coroutine that serves an async HTTP request and
dropped the returned task. The event loop only keeps a weak reference, so the
task could be collected while still pending, leaving the request unanswered
and "Task was destroyed but it is pending!" in the log.
The session already outlives the request in _asyncsessions, so it holds the
task in a set and discards it from a done callback.
nodeconsole spawned a task per input byte from the stdin reader callback
and kept no reference to it. Two of those tasks overlap as soon as one
parks in relay_keypresses waiting on the VNC connection, so keystrokes
can reach the node out of order and the escape sequence state (buffer,
inputcontext, modkeys) is mutated by more than one task at a time. With
the first keystroke relaying slowly, typing abcdef arrives as bcdefa.
Those tasks were also unreferenced, which asyncio documents as
collectable while still pending, so a keypress could be dropped.
Queue the bytes in the reader callback and process them from one
long-lived task instead. Keep a reference to the watch_input task as
well, since collecting that one takes the whole input handler with it.
local_node_trust_setup() called get_cluster_list() and sign_host_key() without
awaiting them, so "osdeploy initialize -l" aborted with "TypeError: cannot
unpack non-iterable coroutine object" before doing any work.
Both awaits have to land together: sign_host_key() is called in a loop that
unlinks the existing ssh_host_*_key-cert.pub before writing the new one, so
fixing only the unpack would delete every host certificate and then fail.
If someone wants to seal to a PCR
explicitly to prevent booting rescue, the PCR is likely to
extend differently during install.
Leave the volume sealed to the tpm without any PCRs until first boot.
Then wipe the bindings without PCR specified, and seal according to user preferred values.
It has been observed there are times where an ethernet switch is partially working with MLD snoop/IGMP snoop. A workaround for the unreliable behavior seems to be to reassert multicast joins ever so often.
Give it a try to restart the SSDP sockets every minute.
The tree is clean under it now, so the job can fail the run and catch the next
async regression instead of only reporting one.
Nothing is suppressed beyond the two ignores that state their reason at the
line, for an async __new__ and an untyped callback registry, neither of which
pyrefly can model.
The plugin was written against the http.client based SecureHTTPConnection, and
when that went away the reference was pointed at the aiohttp WebConnection,
which shares the name and nothing else. Nothing in it could run: the transport
called an async request() without awaiting it and then reached for a
getresponse() the new class does not have, and three PDUClient methods that
were never coroutines were awaited by the entry points.
Two transports now, both local to this plugin. https is aiohttp and stays on
the event loop, since the cert verifier records new fingerprints through
tasks.spawn. http is http.client in a thread, with its own socket so it can
still ask for a smaller segment size before connect: aiohttp only takes a
socket factory from 3.12 on, newer than el9, el10, ubuntu 24.04 or Leap 16
ship. That side has no cert to verify and its credentials arrive already read,
so the thread touches nothing.
connect() establishes and authenticates, wc is just the accessor now, and
logout() no longer sends a session id it never obtained. update() reports an
unsupported element instead of raising NameError.
On the https side cookies follow aiohttp's domain rules and the one POST with
a body goes out as text/plain, where http.client replayed every cookie and
sent no content type. The http side is as before, and neither can be settled
without an Eaton PDU on the bench. Both transports were exercised against a
stand-in: login, outlet read and set, sensors, logout, and a clamped segment
size on the plaintext path.
virEventRunDefaultImpl waits for an event that an idle domain need not
produce, so the thread could outlive a deactivation that reported success, and
every later activation was refused while it did. Registering a timeout is what
makes it return: measured, a thread with nothing registered was still running
four seconds after being asked to stop, and with a half second timer it came
out at once.
Deactivation dropped its reference once the wait expired, whether or not the
thread had stopped, so the next activation started a second one and revived
the first by setting run_console again. The reference is cleared only when the
thread is really gone, and activation refuses with 0x80 while one is alive.
That makes the wait a courtesy rather than a correctness measure, so it drops
to a second.
Activation started an event thread whichever way the base handler had just
answered, so a refusal started one anyway and an already active console got a
second. activated alone cannot tell the two refusals apart, being true
already on the already active path, so the value from before the call decides.
Deactivation joined that thread on the event loop, where it could stall every
other session and its own response. The wait moves off the loop and is
bounded, and the thread is a daemon.
The console awaits its output handler, but both sample BMCs supplied a plain
function, and both dropped the send_data coroutine. virshbmc additionally
receives its stream callback on a libvirt thread, so the send goes through
run_coroutine_threadsafe against the loop captured at activation, called
asyncloop because the class already has a loop method it uses as a thread
target.
ServerConsole asked the session layer to retry, which a ServerSession cannot
do: it never runs Session.__init__, so it has no timeout, and its _timedout
does nothing. IpmiServer.logout was synchronous and an argument short while
_cleanup awaits logout(False). The boot options handler answered, then read an
unbound name and answered again with 0xff.
bmc.py, serversession.py, fakebmc.py and virshbmc.py were byte identical to
upstream pyghmi: the async port went through the session layer beneath them
and left the server side alone. So every response created a coroutine and
dropped it, and the overrides the now async parent awaits returned None.
Running fakebmc bound no socket, spun a core, and answered nothing.
Everything that sends a response is a coroutine now, and so is the dispatch
that reaches it; the hooks a subclass implements stay ordinary functions, so
an out of tree Bmc is unaffected unless it overrides the payload handlers.
Two things had to leave their constructors, both being coroutines: assigning
the server socket, into bind(), which is why listen() is no longer a
classmethod, and answering the open session request, into
send_open_session_response.
Verified against upstream with ipmitool over nine commands, with identical
output. SOL is not covered: fakebmc reports the payload disabled on both.
Housekeeper dates from when this work was a blocking select loop. Under
asyncio the event loop is already that place, and a thread around it takes a
captured loop reference, a scheduling step before the thread starts, and a
daemon flag, only to sit waiting on a coroutine that never returns with no way
to stop it. A task runs on the right loop by construction and cancels. The
loop is looked up before the coroutine is built, so calling this without one
raises rather than stranding it.
Nothing in this tree used the class. Out of tree callers need
start_housekeeping() instead, and gain the ability to stop it.
TsmConsole created an aiohttp ClientSession and never closed it, and leaked it
again when ws_connect failed. Neither was reachable before the connection path
was repaired. It is closed on both paths, and starts as None so that closing
before a connect does not trip over a missing attribute.
A task per read swallowed failures, could deliver keystrokes out of order, and
at end of input returned with the reader still registered, so a level
triggered selector called it again for the same EOF. The reader queues now,
one consumer sends in order, and it is gathered with the main loop so a
failure reaches the caller.
Three faults in the same few lines. The redfish Command lost its constructor
for an async create, so building one raised TypeError, which the except below
reported as TargetEndpointUnreachable. await_redirect is defined nowhere in
this repository's history, so that call raised too; create performs the
session setup it was meant to trigger. And oem is a coroutine method rather
than an attribute, with its web connection coming from get_wc, which is what
performs the login that sets csrftok.
The retry pass calls apply_configuration with lastchance=True, which only
NetworkManager accepts. It is unreachable today, since only NetworkManager
returns the 1 that fills the retry list, but it springs the moment either of
the others grows a return, or the retry selection is brought in line with the
first pass. Matching the signatures costs nothing.
ConsoleSession grew an async create and lost its constructor, and ShellSession
inherits that. sockapi was updated for the console branch but not the shell
branch immediately below it, so opening a shell session raised TypeError. It
is the only place in the tree that builds one.
Command.eventloop called wait_for_rsp with no timeout. With nothing waiting or
being kept alive there is no deadline to derive one from, so it returns
without suspending and the loop runs flat out, measured at over 100000
iterations in two tenths of a second.
MAX_IDLE gives it something to wait on, as a ceiling rather than a fixed
delay: real deadlines still shorten it and an arriving packet still ends it
early.
The Console was never connected, so main_loop ran against a session that had
never been established. Input arrived on a thread that called send_data and
dropped the coroutine; the thread is gone and the loop watches stdin with
add_reader instead. The output handler was a plain function that
Console._print_data awaits.
Command lost its constructor for an async create classmethod, so the utility
raised TypeError before connecting. The onlogon callback it was built around
is gone as well: create establishes the session itself, so both the callback
and the eventloop that waited for it are unnecessary.
docommand awaited nothing, so every operation produced a coroutine that was
printed and dropped, and three of its calls are async generators. Each BMC is
handled in turn now rather than only the last.
Console.main_loop drives Session.wait_for_rsp, which is a coroutine, so it
spun without ever waiting for a packet. It is a coroutine now, and pyghmicons
runs its main under asyncio.run. pyghmiutil had the same shape around
Command.eventloop.
sockapi sent its collective refusal without awaiting tlvdata.send. redfish
handle_sensors returned a coroutine from most branches and None from the short
ones, so the caller's await raised TypeError; it is a coroutine throughout
now. console send_payload waited for a response without awaiting the wait.
Two suppressions are pyrefly limitations rather than bugs: Session defines an
async __new__, and the keepalive registry holds coroutine functions in an
untyped dict.
Taken from fix/asyncio-port-critical, limited to what pyrefly reports.
The SMM handler still used the httplib style connect/request/getresponse that
the async webclient does not have, so nothing was ever sent. It goes through
grab_response_with_status now, with allow_redirects=False to preserve httplib
behaviour, hence the new webclient parameter. That also fixes a login passing
its headers as urlencode's second positional argument.
Delta PDU's logout was a plain function both callers awaited, so the power
paths raised TypeError. XCC's set_system_configuration was the last
synchronous implementation of a method every caller awaits.
These __main__ blocks called coroutines as if they were functions, so they did
nothing at all. Single calls go through asyncio.run; sshutil, proxmox and
vcenter needed an _selftest coroutine. Two were invisible to pyrefly because
repr() and list() count as using the result: vcenter's get_vm_serial needs an
await, and proxmox's get_vm_inventory is an async generator.
lldp called _extract_neighbor_data twice, once correctly, so the bare call is
dropped. xcc3.remote_nodecfg is not a self test: every other handler defines
it as a coroutine and selfservice.py awaits it.
Catches unawaited coroutines and awaits on non-awaitables, which ruff and
compileall cannot see. Only those two kinds are enabled, and in pyrefly.toml
rather than a --only flag so CI, local runs and editors agree; a full check
reports thousands of errors on this largely unannotated tree.
search-path is what lets imports resolve, and without it a large share of the
findings, including everything in the SMM handler, goes unreported. Pyrefly
walks *.py only and a glob does not lift that, so extensionless tools are
named individually. No baseline: it matches by file, kind and column, so a new
mistake at the same indentation as an old one would pass unnoticed.
Advisory until the existing findings are dealt with.
lxml refuses a str carrying an encoding declaration, and the SMM declares one
when it answers /data/login, so _webconfigcreds has raised ValueError on the
first thing it does after logging in ever since the switch to lxml. stdlib
ElementTree took the same input, which is why it went unnoticed. fromstring
already means to take either shape, so encode there. Confirmed against a
DW612S.
compileall only ever compiles *.py. Handed anything else, even by name on the
command line, it skips the file and still exits 0, so the job has been
checking 218 of the 298 Python files in this tree and reporting success for
the rest. Everything without the extension went unchecked: the whole of
confluent_client/bin and confluent_server/bin, the osdeploy deploy scripts,
the loose misc utilities and the setup.py.tmpl templates.
Those files now go to py_compile, which compiles what it is given. The list
is built from python shebangs, read with the shell builtin rather than by
forking head and grep per file, plus four patterns for the files that carry
no shebang at all and so cannot be detected: the setup templates, configbmc,
add_local_repositories and misc/filterpasswd. It comes to the same 298 files
ruff.toml arrives at through extend-include, and wants keeping in step with
it.
The shebang test matches python anywhere in the line rather than after a
slash or space, because several tools use /usr/libexec/platform-python.
Adds the checks whose findings were cleared in the two preceding commits
(F401, F541, E701, E711, E712, E713, PLC0414) plus three that were already
at zero and cost nothing to lock in: E401, B015 and B023.
B905 is deliberately left out even though it also reads as clean: it only
reports on py310+, and satisfying it would mean adding a keyword the oldest
interpreters this tree runs on cannot parse.
Hand written rather than autofixed, since three of the four need the
surrounding code read to be sure they are equivalent:
- confetty: `powerstate == None` -> `is None`.
- nodeconfig: `setmode != True` / `!= False` -> `not setmode` / `setmode`.
Safe because setmode only ever holds None, True or False, and the two
lines above each test normalise None away first.
- pam: split two `if cond: stmt` one-liners.
- imgutil: `from shutil import copytree as copytree`, an alias that renames
nothing. Not a re-export marker, this is a script.
Entirely mechanical, produced by `ruff check --fix --select F401,F541,E713`
and reviewed rather than taken on faith: deleting an import is only safe if
nothing imports it for its side effects or re-exports it. None of the 19
removed names is referenced anywhere in its file, none appears in any string
literal, and none of the touched files uses eval, exec, globals() or
__import__, so there is no dynamic lookup that could reach them.
The enforced rule set is deliberately narrow: undefined names, statements
in impossible positions, duplicate definitions, invalid escapes and a
couple of bugbear checks that only fire on genuine defects. No style
rules, and the tree is clean under it as of the preceding commits.
Discovery needs help. Ruff only walks *.py, and about a quarter of the
Python here has no extension: every node* CLI tool, the server bin tools,
the osdeploy scripts (some of which carry no shebang either) and the
setup.py templates. extend-include lists them, and *.sh is excluded so
the shell scripts sharing those directories are not parsed as Python.
The CI job pins both the action and the ruff version, since there is no
pyproject.toml for the action to read a version from and an unpinned
`latest` would let a new ruff release fail an unchanged branch.
The guard evaluated the `next` builtin and discarded it, which does
nothing, so a pending node with no handler fell through to
None.NodeHandler(...). The AttributeError was caught by the enclosing
except and logged as "Unexpected error during discovery", turning a node
that should have been quietly skipped into a spurious error in the log.
load_plugins() used `plugin` as the loop variable for plugin file names,
which shadows `import confluent.plugin as plugin` for the whole function.
Nothing in the function needed the module, so this was latent rather than
broken, but the next line that does need it would have failed oddly.
Markus Hilger
Jarrod Johnson
Jarrod Johnson