Skip to content

Conversation

@Kufat
Copy link
Collaborator

@Kufat Kufat commented Jan 31, 2021

I had to rush this along when my original prototype IP scoring bot (which was a very, very preliminary version of this) dies because of a WSL hiccup and I was unable to restart it. It has all the basic features I wanted for both IP and email checking, mainly scoring and a persistent database so we don't waste queries (5k/mo quota.)

Haven't tested this yet, although I did make sure it parsed and loaded on a sopel instance that wasn't connected to anything.

@Kufat Kufat requested review from DoctaMag and danieltharp January 31, 2021 00:30
Copy link
Member

@emmiegit emmiegit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this looks good.

There are some areas where my lack of familiarity with sopel hinders this review, and a lot of my points are nitpicks. There's a lot more here than I saw previously, so it makes sense that things might be structured a bit weird for a time while everything is coming together. There aren't any big red flags or issues, and with safe mode as a thing I think the rollback will be pretty smooth.

# RPL_YOUREOPER because that functionality is restricted to opers.
rand = str(randint(0, 999))
while rand in who_reqs:
rand = str(randint(0, 999))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better as a helper function, generate_request_id() or something

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied from other sopel code. Will fix eventually

if trigger.is_privmsg and ( trigger.account is None or trigger.account.lower() != "kufat" ):
return
full = ( ( trigger.sender.lower() in ("#skipirc-staff", "#kufat") ) or
( trigger.is_privmsg and trigger.account.lower() == "kufat" ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should definitely be constants or in configuration files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

except socket.gaierror:
return bot.say("[IP/Host Lookup] Unable to resolve IP/Hostname")
else:
return bot.say("[IP/Host Lookup] Unable to resolve IP/Hostname")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if-elif-else block seems like it would make sense as a helper function? resolve_ip_info() or such

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is lightly modified stock code, but I agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants