Add reload command to update database from slack #80
Conversation
handlers/challenge_handler.py
Outdated
| def execute(self, slack_wrapper, args, channel_id, user_id): | ||
| """Execute the Reload command.""" | ||
|
|
||
| slack_wrapper.post_message(channel_id, "Start updating CTF challenge handler...") |
There was a problem hiding this comment.
Should we start using more user-friendly messages such as :
Updating CTFs and challenges...
Update finished.
There was a problem hiding this comment.
The reason I ask is because most people don't know/care about the concept of "handlers" :)
There was a problem hiding this comment.
Sure thing. That's why "localization" should be done by someone other than a developer :)
Will update this later. Maybe we should refactor all the texts out of the code in something like a resource file, separating code and texts.
handlers/challenge_handler.py
Outdated
| Initialize the challenge handler. | ||
| This might be refactored. Not sure, if the handler itself should do the channel handling, | ||
| or if it should be used to a common class, so it can be reused. | ||
| Reloads the ctf and challenge information from slack. |
There was a problem hiding this comment.
Imperative: "Reload" not "Reloads"
handlers/challenge_handler.py
Outdated
| pickle.dump(database, open(ChallengeHandler.DB, "wb+")) | ||
|
|
||
| def init(self, slack_wrapper): | ||
| """ |
There was a problem hiding this comment.
Minor: Put into one line.
There was a problem hiding this comment.
We also probably don't need doc strings for things as obvious as this.
There was a problem hiding this comment.
It's a remnant from the previous code.
handlers/challenge_handler.py
Outdated
| "archivectf": CommandDesc(ArchiveCTFCommand, "Archive the challenges of a ctf", None, None) | ||
| } | ||
|
|
||
| def init(self, slack_wrapper): | ||
| def update_database_from_slack(slack_wrapper): |
There was a problem hiding this comment.
Needs self or @classmethod or @staticmethod decorator.
| pickle.dump(database, open(ChallengeHandler.DB, "wb+")) | ||
|
|
||
| def init(self, slack_wrapper): | ||
| ChallengeHandler.update_database_from_slack(slack_wrapper) |
There was a problem hiding this comment.
init is a proper method. So this could use self.update_database_from_slack.
We should change the registration of handlers so that we do NOT instantiate the handler class, in which case we'd change all of these to class or static methods.
I'll file a separate ticket.
| pickle.dump(database, open(ChallengeHandler.DB, "wb+")) | ||
|
|
||
| def init(self, slack_wrapper): | ||
| ChallengeHandler.update_database_from_slack(slack_wrapper) | ||
|
|
||
| # Register this handler | ||
| HandlerFactory.register("ctf", ChallengeHandler()) |
There was a problem hiding this comment.
See that the challenge handler is instantiated here. We probably don't need to if we are keeping all state in the class object itself.
There was a problem hiding this comment.
I just made this "static", so we can call it from the ReloadCommand, which has no reference to the challenge handler.
There was a problem hiding this comment.
Yeah I filed #86 to figure out how we want to do this. This is fine for now.
Moved the database initialization into a static function, so we can trigger it from a challenge command. Calling
!reloadwill reinitialize the pickle database with the information from slack (likely the same as restarting the bot).Fixes #78