fix: remove eventlet#210
Conversation
Replace eventlet.greenpool with concurrent.futures.ThreadPoolExecutor in the BMC discovery script, using as_completed() for proper exception propagation and main-thread result aggregation to avoid race conditions. Remove dead eventlet socket compatibility code (.fd attribute checks) from the IPMI session layer, and clean up stale eventlet references in comments across the codebase. Closes: xcat2#197
Drop python3-eventlet from the Ubuntu Noble build Dockerfile. Clean up remaining greenthread/greenlet terminology in comments across aiohmi IPMI modules, consoleserver, macmap, and the IPMI plugin. Remove a commented-out GreenPool reference in macmap.
Drop NullLock rationale comment (referenced removed library), consoleserver greenthread spawn comment, and update syncfiles CalledProcessError workaround comment.
The repr() check existed because eventlet broke normal exception catching. With eventlet removed, catch CalledProcessError directly.
jjohnson42
left a comment
jjohnson42
left a comment
There was a problem hiding this comment.
Broadly speaking nothing should functionally change as the work for making it work was already done, but have a couple of things to check.
| import os | ||
| import struct | ||
| import subprocess | ||
| import pyghmi.util.webclient as webclient |
There was a problem hiding this comment.
I'd be content to leave this misc example behind, but if wanting to change it, would suggest reworking it to aiohmi.util.webclient instead
There was a problem hiding this comment.
Will take a look at this.
| # if in eventlet, go for the true sendto, which is less glitchy | ||
| intsock = intsock.fd | ||
| intsock.sendto(b'\x01', (myself, iosockets[0].getsockname()[1])) | ||
| iosockets[0].sendto(b'\x01', (myself, iosockets[0].getsockname()[1])) |
There was a problem hiding this comment.
I can try, but wanted to know if this was explicitly tested with real stuff, or was the commentary LLM generated indicating a test?
There was a problem hiding this comment.
That was tested on real hardware. I've used an AlmaLinux 10.1 VM on libvirt/KVM and connected to three different systems: an old Supermicro X10 server, Dell PowerEdge R7525, and IBM Power AC922. IPMI, iDRAC and OpenBMC, respectively.
I can do specific tests if you want.
|
@jjohnson42 I've changed the code as requested. Regarding the test matrix, I could test IPMI connections on those systems:
If there are more tests to do, I can try here. I only don't have more 'kind of' machines. |
2 participants
Hi @jjohnson42, this is my first contribution to Confluent. Taking a shot at #197 with the following changes:
I installed Confluent in a VM and verified the IPMI paths this PR touches still work as expected, but I'd appreciate your eyes on anything I might have missed.
Closes #197