MECHA-253: Implement !reopen without refreshing all cases #260
Conversation
MHajoha
changed the title
There was a problem hiding this comment.
Please remove the usage of a bare exception, catching everything can make debugging really difficult, obscuring what is a runtime error from what is an interpretation error.
Further, testing of this modification resulted in an API error, il look into if this was caused by the modification, a misconfiguration in my mecha environment, or an API bug.
sopel_modules/rat_board.py
Outdated
| try: | ||
| addNamesFromV2Response(result['included']) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Do not use a bare except block.
Further, why are we catching bare exceptions and not handling them?
There was a problem hiding this comment.
Yep I agree that this is terribly bad style of code. This is just how this is done in the rest of the code and I couldn't (be bothered to) figure out what addNamesFromV2Response actually throws.
There was a problem hiding this comment.
From what i can tell it shouldn't be raising an exception. and if it does methinks we would like to know about it. Catch/pass is awful code style. I would prefer if mecha raised an error rather than silence it, that's pretty much the only indication we get if something is broken.
There are a lot of things you may discover in mecha2's codebase that are just plain bad practice.
For now it would be appreciated that any method you introduce/modify you make attempt to clean up.
We will just have to maintain mecha2 until mecha3 is ready to go. (which may be a while)
There was a problem hiding this comment.
Yeah you're right, I'll have a look at fixing that except clause tomorrow.
There was a problem hiding this comment.
Also, is Mecha3 already on its way to becoming a thing? As in a complete rewrite against pydle or similar?
|
I have started on the Mecha 3 project, though it's a long way from
fruition. I'm currently experimenting with the `pydle` framework but I have
not done enough to warrant making a commit to this repo (quick and dirty
hacks need to get cleaned up first).
Once I have more of the Mecha 3 core down I'll commit so others can
contribute.
That said, my focus is on maintaining Mecha 2 and expanding it's features
as such requests/needs arise. Mecha 2 is to remain supported until Mecha 3
is ready too replace it seamlessly
…On Jan 31, 2018 16:41, "Maximilian Haye" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In sopel_modules/rat_board.py
<#260 (comment)>:
> @@ -1648,9 +1648,19 @@ def cmd_reopen(bot, trigger, id):
"""
try:
result = callapi(bot, 'PUT', data={'status': 'open'}, uri='/rescues/' + str(id), triggernick=str(trigger.nick))
- refresh_cases(bot, force=True)
- updateBoardIndexes(bot)
- bot.say('Reopened case. Cases refreshed, care for your case numbers!')
+ try:
+ addNamesFromV2Response(result['included'])
+ except:
+ pass
Also, is Mecha3 already on its way to becoming a thing? As in a complete
rewrite against pydle or similar?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#260 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC94SkCZ2aCJEFg1hBR97AN3v7iPsv9iks5tQQglgaJpZM4RvWLR>
.
|
|
Sounds great. I've been writing around at such a thing myself, though more out of personal interest than anything else. Perhaps you could make what you've done so far a repo of it's own for now, because I'd love to contribute. |
Been toying around with giving only the reopened case a new index rather than remaking the whole board from the API, but I have no way of testing my code.
See MECHA-253