Closed psychomario closed 9 years ago
@psychomario I just pushed a large commit in #59 which, coincidentally, has a lot of the same enhancements you have. I'm wondering if I should just make another commit and incorporate what I'm missing from this or if there's a cleaner way...
That's very unfortunate timing, I did check before I started, but you hadn't pushed then.
You have a lot more changes than me so it'd probably be best to discard this PR but go through and add any changes you've missed.
@psychomario it's not your fault lol it's fine. Leave this open for now and I'll push another commit later on with whatever I'm missing and then we'll close this. Cool?
Sounds good to me.
This branch has been duplicated in #59, so I'm going to close and delete this PR.
I've done most of the non-intrusive items from #59. Below is the copied list of items from #59 which are fixed in this PR.
Bugs
pypxe-server (1/3)
Using iPXE:
debug message has at
before it from when it was\t
DHCP (1/1)
craftOptions
for iPXE chainload we shouldn't have a leading/
to match everything elseMisc (1/3)
open
s should have modeb
as well. (Note, this is a good idea even without Windows as a target)Cleanup
Logging (2/4)
self.logger.debug(' HTTP...
. We don't necessarily need to specify HTTP as we have%(name)s
in the logger format string.%(name)s
and[%(levelname)s]
should probably be swapped, to more strongly associate message with source.DHCP (5/8)
Invalid client request received
vs.Valid client request received
, I don't think this is totally true. The client request isn't invalid, it's just not a PXE request. This should probably be changed to reflect this.if self.http and not self.ipxe
"WARNING" is unnecessary as we're already logging to warningtlvParse
struct.unpack
lines are cleaner as e.g.[tag] = struct.unpack('B', raw[0])
craftOptions
if arch == 0:
should beelif
, also possibly first to order by arch IDdhcp-proxy
HTTP (4/5)
HTTP File Sent
debug log line at the end of HTTPD has an inconsistent addr output to the rest of the project. Should just be{}
HTTP Recieved
debug at the top ofhandleRequest
does an unneccessaryrepr
on the addressHTTPD.handleRequest
, the lines assigningstartline
,method
andtarget
can be inlined and use tuple unpacking instead. The RFC allows this.Content-Length
formatting ->{}
and.format()