New module, speccy.py

[Jessexd]

Features:

Parses speccy links, e.g. http://speccy.piriform.com/results/IX6JcGoJaJhzinvBpyLBtbm and spits out relevant info.

Sample responses for various links:

<bot> (nick) OS: Windows 10 Home 64-bit ● RAM: 8.00GB ● CPU: Intel Core 2 Quad Q9400 @ 2.66GHz Yorkfield 45nm Technology ● GPU: NVIDIA GeForce 9800 GT (PNY) ● Badware: None ● Failing Drive(s): Disk: #1

<bot> (nick) OS: Windows 10 Education 64-bit ● RAM: 16.0GB Dual-Channel Unknown @ 1069MHz (15-15-15-36) ● CPU: Intel Core i7 6700K @ 4.00GHz: 26 °C Skylake 14nm Technology ● GPU: 4095MB NVIDIA GeForce GTX 970 (MSI): 58 °C ● Badware: Killer Service V2 ● Failing Drive(s): None

<bot> (nick) OS: Windows 10 Pro 64-bit ● RAM: 8.00GB Dual-Channel DDR3 @ 799MHz (11-11-11-28) ● CPU: AMD FX-8350: 20 °C Vishera 32nm Technology ● GPU: 4095MB NVIDIA GeForce GTX 1050 Ti (Gigabyte): 34 °C ● Badware: AutoPico Daily Restart, Service KMSELDI ● Failing Drive(s): None

Commands:

None as of yet, therefore no help information either. It purely automatically parses speccy links.

[linuxdaemon]

This should use hook.regex() rather than re-implementing the functionality.

[linuxdaemon]

Why use a file here? soup = BS(response.content, 'lxml-xml') would work just fine

[Jessexd]

Going to look into the other two requests, but about this one. This was actually an optimization improvement. If you know a better way lmk.

https://i.imgur.com/G2Si7PP.png

I noticed it also parses the link faster too:

https://i.imgur.com/R1eKAZT.png

test.py = the method you mentioned test2.py = the current method seen in snippet

[linuxdaemon]

Please include your test case. Also the time command shows system resource usage not actual elapsed time. Disk I/O has a lot of waiting since disk I/O is slow. This code is also running in the main thread as you have marked it as a coroutine, so any blockling I/O like an HTTP request or file write will halt the entire bot until it finishes.

[Jessexd]

I don't exactly know why it's faster, it was just a guess that ended up working, but here so you can try yourself:

https://gist.github.com/Jessexd/14990cbadad5e62a22fb590bb25a4f9d

Should I add multithreaded=True ?

[linuxdaemon]

No, you should remove the @asyncio.coroutine decorator on your hook function, which is marking it as a coroutine, which runs in the main event loop.

[Jessexd]

Ah, alright, as for BS, what should be done in terms of parsing links faster instead of writing to disk?

[linuxdaemon]

Generally just pass the content from the response straight in.

import requests

from bs4 import BeautifulSoup

with requests.get(url) as response:
    soup = BeautifulSoup(response.content, 'lxml', from_encoding=response.encoding)

print(soup)

It may not be the fastest option, but it's the standard way as something like disk i/o can be extremely unpredictable.

[linuxdaemon]

This could be replaces with if soup.body.find(): whatever since .find() will return None when it can't find a matching element.

[Jessexd]

redid as per reviewers requested changes

[linuxdaemon]

I recommend compiling the regex once globally and reusing it rather than recompiling it on every call.

[linuxdaemon]

I recommend compiling the regex once globally and reusing it rather than recompiling it on every call.

[linuxdaemon]

I recommend compiling the regex once globally and reusing it rather than recompiling it on every call.

[linuxdaemon]

I recommend compiling the regex once globally and reusing it rather than recompiling it on every call.

[linuxdaemon]

I recommend compiling the regex once globally and reusing it rather than recompiling it on every call.

[linuxdaemon]

As you don't read the body as a stream, specifying stream=True has no effect, The timeout is also quite low, no reason for that to be honest, as even if the request is slow, other things on the bot will continue to run fine. Waiting on a response from a server is very light on system resources,

[linuxdaemon]

nick, message, and chan are unused parameters, no need to include them.

[linuxdaemon]

You'll want to escape those ., otherwise they will match any character.
I also recommend against doing [A-z] as that includes all characters between the upper and lowercase letters, such as [, \, ], ^, _, and `.

[linuxdaemon]

This would be better as re.sub(r"\s+", " ", ' + '.join(data)) as \s covers all whitespace, including CR and LF.

[linuxdaemon]

To check if a list is empty, you should use the __bool__ value, don't check the length as that can be an expensive operation on a large list.
Something like: if z: instead of if len(z) != 0:

[Jessexd]

Hopefully nailed all requested changes and conformed to standards/quality.

[linuxdaemon]

Do not include backup files in the PR.

[linuxdaemon]

This should be replaced with

for i, spec in enumerate(drivespec):
    drives = spec.next_sibling.next_sibling.stripped_strings

Instead of repeatedly calling __getitem__ on the list.

[linuxdaemon]

Avoid repeatedly calling a function like this. It would be better to assign it to a variable before the check.

[linuxdaemon]

if body.find("div", class_="blue clear", text='Physical Memory') returns None here, it will cause an error. You should do a None check like you do in the rest of the function.

[linuxdaemon]

You should do a None check before using the result of .find()

[Jessexd]

Can you clarify/correct what you mean? In this one snippet it's including code for CPU and GPU.

[linuxdaemon]

Comments apply to the line directly above them, so in this case gpu_find

[linuxdaemon]

Might be easier to just set a variable like has_badware = False and set that to True whenever you find something that's considered badware.

[linuxdaemon]

I think this is supposed to be c_drive = c_drive.parent.parent

[Jessexd]

Pushed requested changes

[linuxdaemon]

Why is this .find() duplicated?

[linuxdaemon]

All of the badware logic could probably be collapsed to something like

badware_patterns = [KILLER_RE, REVIVER_RE, BOOSTER_RE, KMS_RE, PICO_RE]
badware = body.find_all("div", text=badware_patterns)
if badware:
    for tag in badware:
        data.append("\x02Badware:\x02" + " " + tag.text)
else:
    data.append("\x02No Badware\x02")

Rather than duplicating the code. Then any new Badware patterns just need to be added to that list, and that list could even be defined with the rest of the regexs like:

BADWARE_PATTERNS = [...]

[Jessexd]

done, lots better

[linuxdaemon]

Why even use values here if you just return the first thing you find?

[Jessexd]

done

[linuxdaemon]

drive_spec will never be None. .find_all() always returns a list, it may just be empty

[linuxdaemon]

drives will always be a string or None here, as you changed how smartcheck() works. If you want to get info for multiple drives, you'll need to change the logic in smartcheck()

[Jessexd]

@linuxdaemon
Here's the change.

[linuxdaemon]

Why is this line continuation needed? It makes the code much more unreadable

[Jessexd]

Yeah, thought it looked better, as I see now It's kind of crowded. I hate those line warnings but I want so much to keep it pep8 acceptable.

[linuxdaemon]

Extraneous whitespace

[Jessexd]

Can't pinpoint what you see, as again, it's since been the same many revisions ago.

[linuxdaemon]

I mean the extra line after the def line. I just didn't catch it in earlier reviews.

[linuxdaemon]

Extraneous comma and whitespace

[Jessexd]

I'm not sure what you're referring to, these 4 lines have been the same since many reviewed (by you) revisions ago.

[linuxdaemon]

Here I'm referring to the comma and space after match.group()

[linuxdaemon]

These three if block could most likely be generalized to a function call since they are almost exactly the same.

[Jessexd]

I'll compress that too.

[linuxdaemon]

Why do you check if drive_spec_bad is True directly after setting it to True?

[Jessexd]

Didn't realize that, tested fine, but I'll remove that certainly.

[linuxdaemon]

Surely you can use a different criteria to find this element. Like matching an specific attribute or something.

[Jessexd]

I'll see how I can compress that where it's less obviously ridiculous.

[linuxdaemon]

I'm not even sure what this is trying to accomplish.

[Jessexd]

Checks if 0000000000 in the grabbed 05 div of HTML. 0000000000 being the raw SMART value. If it's anything that is anything but 0000000000, it's a broken drive. Below would be obviously what to do if it's not 0000000000.

[linuxdaemon]

Why the comparison to 0 though? surely if "0000000000" in drives" is what you want?

[linuxdaemon]

This statement doesn't actually do anything.

[Jessexd]

Hm, thought it did. One less piece of code needed once I test it. 👍

[linuxdaemon]

str is the raw string type. str.strip('0') is equivalent to '0'.strip() and since you don't set the resturn value of that statement to anything, this statement doesn't do anything. What you should do is examine why you originally added this line and see if it was a mistake in case you do still need to strip a string.

[linuxdaemon]

Do you have any intention of continuing work on this plugin?