Allow snekbox to run in both 3.10 and 3.11 by ChrisLovering · Pull Request #2218 · python-discord/bot

ChrisLovering · 2022-07-13T21:24:22Z

This PR updates the snekbox cog to run using Python 3.11 by default, and send a button with the code output response:

Clicking this button allows the use to re-run their same code, but under 3.10.

Clicking this button deletes the previous response and sends a new response, also with the button. Clicking the button again allows the user to change back to 3.11.

This also works with editing your evaled code, where the button ill always run the most recent edition of the user's code.

To do this the cog needed to track, for each user, the active eval's code, python version and response message.
Since a button press can now also trigger a job to continue, if we did not track these then editing your own code and then re-evaluating it would trigger the wait_fors in continue_job for each time you pressed the button to change languages.

MarkKoz · 2022-07-13T22:07:50Z

+                self.jobs[ctx.author.id][ctx.message.id] = {
+                    "code": code,
+                    "python_version": python_version,
+                    "start_date": datetime.datetime.now(),
+                    "bot_response_message_id": response.id
+                }


A namedtuple would be nicer, though I have doubts that you even need to store most of this.

You're correct, it only needs to be a invokation:response mapping 0817398

wookie184 · 2022-07-14T09:03:33Z

Being able to run in 3.11 sounds good.

I'm not sure that the button to change versions is worth the added clutter given that in the vast majority of cases of eval commands the version wouldn't make a difference.

Was having a separate command like !eval3.10, or an optional argument like !eval 3.10 considered?

ChrisLovering · 2022-07-14T09:19:24Z

Being able to run in 3.11 sounds good.

I'm not sure that the button to change versions is worth the added clutter given that in the vast majority of cases of eval commands the version wouldn't make a difference.

Was having a separate command like !eval3.10, or an optional argument like !eval 3.10 considered?

This PR does add optional argument to the commands too.

The thoughts behind the button is if the user runs into a 3.11 problem, they can easily switch to 3.10 and see if it works on that.

ChrisLovering · 2022-07-14T09:23:30Z

Something that could reduce the clutter would be making the delete action a button too, so it's inn line, rather an emoji below.

MarkKoz · 2022-07-16T20:56:58Z

I get the following error when trying to use the delete button

2022-07-16 13:56:31 | discord.ui.view | ERROR | Ignoring exception in view <ViewWithUserAndRoleCheck timeout=180.0 children=2> for item <DeleteMessageButton style=<ButtonStyle.secondary: 2> url=None disabled=False label='Delete' emoji=None row=None>
Traceback (most recent call last):
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/ui/view.py", line 412, in _scheduled_task
    await item.callback(interaction)
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/botcore/utils/interactions.py", line 86, in callback
    await interaction.delete_original_message()
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/interactions.py", line 486, in delete_original_message
    await adapter.delete_original_interaction_response(
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/webhook/async_.py", line 217, in request
    raise NotFound(response, data)
discord.errors.NotFound: 404 Not Found (error code: 10015): Unknown Webhook

ChrisLovering · 2022-07-16T21:07:17Z

I get the following error when trying to use the delete button

2022-07-16 13:56:31 | discord.ui.view | ERROR | Ignoring exception in view <ViewWithUserAndRoleCheck timeout=180.0 children=2> for item <DeleteMessageButton style=<ButtonStyle.secondary: 2> url=None disabled=False label='Delete' emoji=None row=None>
Traceback (most recent call last):
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/ui/view.py", line 412, in _scheduled_task
    await item.callback(interaction)
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/botcore/utils/interactions.py", line 86, in callback
    await interaction.delete_original_message()
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/interactions.py", line 486, in delete_original_message
    await adapter.delete_original_interaction_response(
  File "/home/mark/repos/python/bot-pydis/.venv/lib/python3.9/site-packages/discord/webhook/async_.py", line 217, in request
    raise NotFound(response, data)
discord.errors.NotFound: 404 Not Found (error code: 10015): Unknown Webhook

I keep using the wrong delete method for these interactions 😮‍💨
python-discord/bot-core#104 for the fix.

MarkKoz · 2022-07-16T21:20:01Z

Error

2022-07-16 14:17:41 | botcore.utils.scheduling | ERROR | Error in task Task-674 139899212692800!
Traceback (most recent call last):
  File "/home/mark/repos/python/bot-pydis/bot/exts/utils/snekbox.py", line 466, in run_job
    response = await self.send_job(ctx, python_version, code, args=args, job_name=job_name)
  File "/home/mark/repos/python/bot-pydis/bot/utils/lock.py", line 112, in wrapper
    raise LockedResourceError(str(namespace), id_)
bot.errors.LockedResourceError: Cannot operate on snekbox.run_job `137073073168973824`; it is currently locked and in use by another operation.

Reproduction:

Run an eval
Run another eval
Quickly press "run in 3.10/11" on the first eval before the second finishes.

ChrisLovering · 2022-07-16T21:22:54Z

Hmm, it seems the cog_command_error isn't hit when the function is being called from the interaction.

It's an easy fix, we just need to move the error handling from there to instead be a try/except around send_job inside run_job.

ChrisLovering · 2022-07-16T21:30:50Z

Will squash some of these commits before merging.

MarkKoz · 2022-07-16T22:22:37Z


        return output, paste_link

+    @lock_arg("snekbox.run_job", "ctx", attrgetter("author.id"), raise_error=True)


Not really important but...

Suggested change

@lock_arg("snekbox.run_job", "ctx", attrgetter("author.id"), raise_error=True)

@lock_arg("snekbox.send_job", "ctx", attrgetter("author.id"), raise_error=True)

Good point, fixed this!

To do this we need to track, for each user, the active eval's code, python version and response message. This is because a button press can now also trigger a job to continue. If we did not track these, then editing your own code and then re-evaluating it would trigger the wait_fors in continue_job for each time you pressed the button to change languages. Co-authored-by: Mark <1515135+MarkKoz@users.noreply.github.com>

Rather than passing around superfluous variables.

The cog_command_error isn't hit when the run_job function is called from the button interaction, this means if the lock error is raiseed, it doees not get handled.

This is so that we do not need to spawn the run_job call in a seperate task. This also wraps interaction.message.delete() in a NotFound suppress to cover the case where a user re-runs code and very quickly clicks the button. The log arg on send_job will stop the actual job from running in this case.

ChrisLovering requested review from Den4200, MarkKoz, jb3 and ks129 as code owners July 13, 2022 21:24

ChrisLovering force-pushed the 3.11-snekbox branch from a2c95b4 to 08735a1 Compare July 13, 2022 21:44

jb3 approved these changes Jul 13, 2022

View reviewed changes

MarkKoz requested changes Jul 13, 2022

View reviewed changes

ChrisLovering requested a review from MarkKoz July 16, 2022 16:08

ChrisLovering force-pushed the 3.11-snekbox branch from e5780bc to 7a852f0 Compare July 16, 2022 16:33

MarkKoz requested changes Jul 16, 2022

View reviewed changes

MarkKoz reviewed Jul 16, 2022

View reviewed changes

Comment thread docker-compose.yml

ChrisLovering added a: utility Related to utility commands: (bot, eval, extensions, jams, reminders, snekbox, utils) t: enhancement Changes or improvements to existing features review: do not merge The PR can be reviewed but cannot be merged now labels Jul 16, 2022

ChrisLovering force-pushed the 3.11-snekbox branch from e5006b2 to 6a7d840 Compare July 16, 2022 21:35

MarkKoz reviewed Jul 16, 2022

View reviewed changes

Comment thread bot/exts/utils/snekbox.py

MarkKoz approved these changes Jul 16, 2022

View reviewed changes

Add required config and constants for snekbox 3.11

3ad7cd9

ChrisLovering force-pushed the 3.11-snekbox branch from 0c8f933 to ec8aac5 Compare July 16, 2022 22:40

ChrisLovering and others added 3 commits July 16, 2022 23:45

Update snekbox tests to reflect current behaviour

32d44d1

Use generic views from bot0core for snekbox

c4a7722

ChrisLovering added 3 commits July 16, 2022 23:45

Infer the snekbox invoker from context

b20b72a

Rather than passing around superfluous variables.

Move snekbox lock error handling to a try/except

7e1f74f

The cog_command_error isn't hit when the run_job function is called from the button interaction, this means if the lock error is raiseed, it doees not get handled.

ChrisLovering force-pushed the 3.11-snekbox branch from ec8aac5 to aca5494 Compare July 16, 2022 22:45

ChrisLovering removed the review: do not merge The PR can be reviewed but cannot be merged now label Jul 16, 2022

ChrisLovering merged commit f568618 into main Jul 16, 2022

ChrisLovering deleted the 3.11-snekbox branch July 16, 2022 22:48


		return output, paste_link

		@lock_arg("snekbox.run_job", "ctx", attrgetter("author.id"), raise_error=True)

	@lock_arg("snekbox.run_job", "ctx", attrgetter("author.id"), raise_error=True)
	@lock_arg("snekbox.send_job", "ctx", attrgetter("author.id"), raise_error=True)

Uh oh!

Conversation

ChrisLovering commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarkKoz Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

ChrisLovering Jul 16, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wookie184 commented Jul 14, 2022

Uh oh!

ChrisLovering commented Jul 14, 2022

Uh oh!

ChrisLovering commented Jul 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarkKoz commented Jul 16, 2022

Uh oh!

ChrisLovering commented Jul 16, 2022

Uh oh!

MarkKoz commented Jul 16, 2022

Uh oh!

ChrisLovering commented Jul 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrisLovering commented Jul 16, 2022

Uh oh!

Uh oh!

MarkKoz Jul 16, 2022

Choose a reason for hiding this comment

Uh oh!

ChrisLovering Jul 16, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ChrisLovering commented Jul 13, 2022 •

edited

Loading

ChrisLovering commented Jul 16, 2022 •

edited

Loading