Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Tags] Save data in the config directory #637

Merged
merged 2 commits into from
Apr 8, 2023
Merged

[Tags] Save data in the config directory #637

merged 2 commits into from
Apr 8, 2023

Conversation

quachtridat
Copy link
Member

@quachtridat quachtridat commented Apr 6, 2023 •
edited
Loading

Summary

This PR fixes #638 by making Tags write to the directory specified in Config class (which is specified by the cog to be the cog data path set up as a part of bot instance setup) which is writable.

Details

Currently the cog writes to a file whose path is specified as a purely relative path, which, at absolute path resolution time, will become a path under wherever the bot is running (most likely not the writable cog data path) and the cog will fail when it does not have write permission to the said resolved path.

This commit specifies a path relative to the cog data path which is writable by cogs, which avoids the aforementioned issue.

Testing

Without the fix

image

Log lines 
ren  | [2023-04-08 01:01:05] [ERROR] discord.client: Ignoring exception in on_message
ren  | Traceback (most recent call last):
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 229, in wrapped
ren  |     ret = await coro(*args, **kwargs)
ren  |   File "/data/cogs/CogManager/cogs/tags/tags.py", line 449, in create
ren  |     await self.config.put(location, db)
ren  |   File "/data/cogs/CogManager/cogs/tags/config.py", line 59, in put
ren  |     await self.save()
ren  |   File "/data/cogs/CogManager/cogs/tags/config.py", line 50, in save
ren  |     await self.loop.run_in_executor(None, self._dump)
ren  |   File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 58, in run
ren  |     result = self.fn(*self.args, **self.kwargs)
ren  |   File "/data/cogs/CogManager/cogs/tags/config.py", line 40, in _dump
ren  |     with open(temp, "w", encoding="utf-8") as tmp:
ren  | PermissionError: [Errno 13] Permission denied: '84f4e12f-9800-4e24-a760-f2b487f19409-tags.json.tmp'
ren  |
ren  | The above exception was the direct cause of the following exception:
ren  |
ren  | Traceback (most recent call last):
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/client.py", line 409, in _run_event
ren  |     await coro(*args, **kwargs)
ren  |   File "/data/venv/lib/python3.9/site-packages/redbot/core/events.py", line 374, in on_message
ren  |     await bot.process_commands(message)
ren  |   File "/data/venv/lib/python3.9/site-packages/redbot/core/bot.py", line 1599, in process_commands
ren  |     await self.invoke(ctx)
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 1353, in invoke
ren  |     await ctx.command.dispatch_error(ctx, exc)
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 633, in dispatch_error
ren  |     await injected(cog, ctx, error)
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 211, in wrapped
ren  |     ret = await coro(*args, **kwargs)
ren  |   File "/data/cogs/CogManager/cogs/tags/tags.py", line 469, in create_error
ren  |     raise error
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 1349, in invoke
ren  |     await ctx.command.invoke(ctx)
ren  |   File "/data/venv/lib/python3.9/site-packages/redbot/core/commands/commands.py", line 782, in invoke
ren  |     await super().invoke(ctx)
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 1642, in invoke
ren  |     await ctx.invoked_subcommand.invoke(ctx)
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 1023, in invoke
ren  |     await injected(*ctx.args, **ctx.kwargs)  # type: ignore
ren  |   File "/data/venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 238, in wrapped
ren  |     raise CommandInvokeError(exc) from exc
ren  | discord.ext.commands.errors.CommandInvokeError: Command raised an exception: PermissionError: [Errno 13] Permission denied: '84f4e12f-9800-4e24-a760-f2b487f19409-tags.json.tmp'
With the fix

image

@quachtridat
[Tags] Save data in the config directory
bc4a0f2 
Make `Tags` write to the directory specified in `Config` class (which is
specified by the cog to be the cog data path set up as a part of bot
instance setup) which is writable.

Currently the cog writes to a file whose path is specified as a purely
relative path, which, at absolute path resolution time, will become a
path under wherever the bot is running (most likely not the writable cog
data path) and the cog will fail when it does not have write permission
to the said resolved path.

This commit specifies a path relative to the cog data path which is
writable by cogs, which avoids the aforementioned issue.
@quachtridat quachtridat added the bug Umbrella label for things that shouldn't happen and should be addressed relatively soon. label Apr 6, 2023
@quachtridat quachtridat mentioned this pull request Apr 8, 2023
Closed
Injabie3
Injabie3 approved these changes Apr 8, 2023
View reviewed changes
Copy link
Member

@Injabie3 Injabie3 left a comment

Choose a reason for hiding this comment

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

Ship it!

@quachtridat quachtridat merged commit bc3871d into SFUAnime:V3/testing Apr 8, 2023
@quachtridat quachtridat deleted the quachtridat/issue000-tags-fix-config-dump-dir branch April 8, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Injabie3 Injabie3 Injabie3 approved these changes

Assignees
No one assigned
Labels
bug Umbrella label for things that shouldn't happen and should be addressed relatively soon.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Tags] PermissionsError when Ren runs in Docker container
2 participants
@quachtridat @Injabie3