Completion codes 201 and 202 mean the chunk asked for was too big, and the
check for them sat after a call that raises, so a bmc that cannot serve 224
bytes at once failed the fru read rather than being asked for less.
The retry could not terminate either: chunksize // 2 + 2 is 4 for a chunksize
of 4, so the chunksize == 3 guard was unreachable and a bmc that kept refusing
would have been asked for 4 bytes for ever.
Completion code 203 on a sensor reading means the sensor is not present, which
is expected of an optional device, but the check for it sat after a call that
raises first, so one absent sensor ended the whole sensor sweep.
_supports_standard_ipv6 read rsp['code'] the same way, so it could only ever
answer True; a platform without the standard parameters raised instead. A
completion code is that platform's answer, while a timeout or a lost session is
not and must not be cached as one.
raw_command's docstring still described itself as the other call it was renamed
from, which is how these checks came to be written against the wrong contract.
raw_command raises on any nonzero completion code, so the 0xCA and 0xC5 checks
in get_sdr could not run: a bmc that will not return a whole record in one go,
or whose reservation went stale, failed the sensor load outright instead of
being retried. Read the code from the exception, which carries it.
The back off also had a fixed point, size // 2 + 2 being 3 for a size of 3, so
a bmc that kept refusing would have been asked the same question for ever.
Give up when the request cannot get any smaller. A header read cannot go
under 5 bytes either, since that is where the record length sits, so give up
there rather than parse a reply too short to index.
The stale reservation retry could not end on its own either: it cleared the id
and left taking a new one to the top of the loop, which only reserves for a
partial read. Take one where the code is handled.
Closing a console gives up its claim on the session, and that talks to the
bmc, so let it fail the same way the console's own deactivate is already
allowed to.
kg was left as the caller passed it in both the register key and the reuse
check, so the mismatch fixed for the name and password still applied to it.
The count for a new socket was taken before binding it and before the io
task was known to be up. Take it last, once nothing is left that can still
fail.
Telling a keepalive that the session is gone can end up back in logout,
because reporting it is how a console gives up its claim. The inner pass
finished by clearing the register of keepalives while the outer was still
walking it, so the next entry was looked up on None. Two entries is what an
XCC has, the console's and the oem handler's.
Take the callbacks and give up the register before notifying anyone.
The check for sharing a live session compared the credentials a session
keeps encoded against the strings every caller passes, so it never matched
and each caller built another session beside the one it could not see.
Normalise both sides. Three commands to one bmc went from three sessions on
three sockets to one, and from five sessions open on the bmc to three.
Not from the port to asyncio: upstream compares the same two things the same
way.
One session is now routinely handed to a console and a command at once, and
logout closed it for both, leaving whoever was left holding one that
answered as though it had been lost. Count the holders and give up a claim
instead, unless the session is no longer usable, which logout is told by
sessionok.
A console had no way to give a claim back: close deactivated its sol payload
and left the session alone, which was right when closing meant closing it
for everybody and is a leak now. Both of its exits release it.
__init__ opened by checking for an initialized attribute, meaning it had
been handed a session someone else was establishing. That attribute is only
ever set further down in the same method, so the check cannot be true, and
the port lost the return that made it work upstream. Waiting for someone
else's login is done in __new__ now, so this is dead code claiming to
protect something.
logout decremented the count of sessions on a socket twice over, and
_mark_broken again for the case logout had not, so it went negative and kept
falling. _assignsocket picks the least used socket and refuses one at
MAX_BMCS_PER_SOCKET, and both of those read that number.
initting_sessions exists so a caller can share a session already on its way,
but the entry was added only once the login had finished, leaving the login
itself uncovered. Two callers asking at once each built a session, both on
the socket the other had not claimed yet, and replies route by bmc address
and local port, so only the last to transmit was ever answered. That is the
console session failing about one attempt in three.
Register before the login and remove the entry in a finally. The two old
removals keyed on the encoded credentials while the register is keyed on the
caller's strings, so they never matched and an entry outlived its session.
A session handed over mid login is now waited for rather than returned as
one that answers as though it had been lost.
A session that failed raised with no message at all, so a failed console
read "IpmiException: None". Record the reason wherever a session is marked
broken.
The asyncio rework mistakenly follows up a long waiting acquire with blanking and starting over.
Now use 'True' as a sentinel value to trigger a fetch, otherwise, assume srp is ripe for processing.
Immediately discard srp after unpack and replace with sentinal value.
Fifteen error kinds beyond the two async ones report nothing on this tree
today and have something in it to bite on, so turning them on costs no
findings and keeps it that way.
invalid-syntax is the one that closes a gap rather than covering ground
another check already holds: CI compiles under a modern interpreter, which
accepts syntax the el8 and sles15 interpreters cannot parse. Checking
against python-version rejects it instead, which makes that setting load
bearing for the first time.
Kinds whose subject matter this tree does not contain stay off, among them
everything reached only through typing: the module is never imported, so
TypeVar and namedtuple naming and stale `# type: ignore` have nothing to
find here.
Every rule added here is at zero once the previous commit lands, so it costs
no cleanup: the point is that a future patch cannot introduce one without the
ruff job failing. They are the rest of pyflakes' format-string checks,
flake8-2020, most of bugbear, the pylint warnings that describe bugs rather
than style, four flake8-async rules for blocking calls in coroutines, and
some RUF, LOG, PGH, PIE, ISC and EXE rules in the same spirit. Each was
confirmed to fire on a synthetic violation, so none is silently inert under
the py37 target.
Rules are named by group wherever the group is already clean, and each prefix
stops short of a rule that is not: PLW150 rather than PLW15, which would pull
in PLW1510. Bugbear is listed rule by rule apart from B02 and B03, since
B006, B007 and B018 are all left out on purpose.
bin/confluentsrv.py is a python2 script that setup.py never lists in
scripts, so no package has ever installed it, and both the systemd unit and
the sysvinit script start bin/confluent instead. It had also drifted out of
step with what it calls: main.run takes the argument vector and this passed
none.
confluentsrv.spec goes with it. It is a PyInstaller spec whose only input
is c:/Python27/Scripts/confluentsrv.py, a path that has never existed in
this tree, left over from the Windows compatibility work.
WebConnection requires a port, and every other caller passes one, twice in
this very file. Following a relay URL during discovery raised TypeError
instead.
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.
Each is the only thing keeping its rule group from being selectable whole.
userutil.py imported ctypes with a star; the names it uses are POINTER,
byref, c_char_p, c_int, c_int32, c_uint and cdll. The oem lookup loop had an
else with no break, so the else always ran. The alert parameter table wrapped
int in a lambda that only forwards to it. And the watchdog interval passed 0
where os.environ.get documents a string, which worked because int(0) is 0.
Jarrod Johnson
Markus Hilger
Jarrod Johnson