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

Properly handle missing schemas/tables in PostgreSQL driver #5855

Merged
merged 8 commits into from Oct 13, 2022
Merged

Properly handle missing schemas/tables in PostgreSQL driver #5855

merged 8 commits into from Oct 13, 2022

Conversation

Jackenmen
Copy link
Member

Description of the changes

  • Fixes Red is spamming postgres with an invalid query  #3983
    • The driver worked as designed however, we've decided that it would be better if we revised this design to avoid executing erroneous SQL queries
    • I've also added additional verification step in the PostgreSQL tests job to make sure we don't suppress any such errors
  • Fixes an edge case in the PostgreSQL driver that could occur when trying to clear the whole Config scope when there's no config data for (cog_name, cog_id) pair:
.tox/postgres/lib/python3.8/site-packages/redbot/core/config.py:386: in clear_raw
    await self.driver.clear(identifier_data)
.tox/postgres/lib/python3.8/site-packages/redbot/core/drivers/postgres/postgres.py:167: in clear
    await self._execute(
.tox/postgres/lib/python3.8/site-packages/redbot/core/drivers/postgres/postgres.py:237: in _execute
    return await method(query, *args)
.tox/postgres/lib/python3.8/site-packages/asyncpg/pool.py:530: in execute
    return await con.execute(query, *args, timeout=timeout)
.tox/postgres/lib/python3.8/site-packages/asyncpg/connection.py:317: in execute
    _, status, _ = await self._execute(
.tox/postgres/lib/python3.8/site-packages/asyncpg/connection.py:1639: in _execute
    result, _ = await self.__execute(
.tox/postgres/lib/python3.8/site-packages/asyncpg/connection.py:1664: in __execute
    return await self._do_execute(
.tox/postgres/lib/python3.8/site-packages/asyncpg/connection.py:1711: in _do_execute
    result = await executor(stmt, None)
asyncpg.exceptions.InvalidSchemaNameError: schema "PyTest.32127" does not exist
  • Adds more tests for getting and clearing data on custom config scopes with partial identifiers

Have the changes in this PR been tested?

Yes

Proof that the added tests can fail:
https://github.com/jack1142/Red-DiscordBot/actions/runs/3162818621/jobs/5149778704#step:6:58

@Jackenmen Jackenmen added Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. Type: Enhancement Something meant to enhance existing Red features. hacktoberfest-accepted Used to mark a PR as valid Hacktoberfest contribution. DO NOT REMOVE UNTIL END OF NOVEMBER (sic!)! labels Oct 1, 2022
@Jackenmen Jackenmen added this to the 3.4.19 milestone Oct 1, 2022
@Jackenmen Jackenmen requested a review from Kowlin as a code owner October 1, 2022 02:24
@github-actions github-actions bot added Category: CI This is related to repository's CI configuration. Category: Tests labels Oct 1, 2022
Kowlin
Kowlin approved these changes Oct 13, 2022
View reviewed changes
Copy link
Member

@Kowlin Kowlin left a comment

Choose a reason for hiding this comment

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

Tests are passing, bot runs fine in pSQL from what I've seen, feel free to merge when ready.

@Jackenmen Jackenmen merged commit a3de616 into Cog-Creators:V3/develop Oct 13, 2022
1 check passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Oct 13, 2022
@Jackenmen Jackenmen deleted the tests_and_postgresql_driver_improvements branch October 13, 2022 11:38
Jackenmen added a commit to Jackenmen/Red-DiscordBot that referenced this pull request Nov 1, 2022
@Jackenmen
Properly handle missing schemas/tables in PostgreSQL driver (Cog-Crea…
ec7b0db 
…tors#5855)
Drapersniper pushed a commit to Drapersniper/Red-DiscordBot that referenced this pull request Nov 27, 2022
@Jackenmen @Drapersniper
Properly handle missing schemas/tables in PostgreSQL driver (Cog-Crea…
7396af8 
…tors#5855)
Drapersniper pushed a commit to Drapersniper/Red-DiscordBot that referenced this pull request Dec 2, 2022
@Jackenmen @Drapersniper
Properly handle missing schemas/tables in PostgreSQL driver (Cog-Crea…
b30c1a2 
…tors#5855)
Red-GitHubBot pushed a commit to Red-GitHubBot/Red-DiscordBot that referenced this pull request Apr 19, 2023
@Jackenmen @red-githubbot
[3.4] Properly handle missing schemas/tables in PostgreSQL driver (Co…
d7e6ddb 
…g-Creators#5855)

(cherry picked from commit a3de616)

Co-authored-by: Jakub Kuczys <me@jacken.men>
@red-githubbot
Copy link

red-githubbot bot commented Apr 19, 2023

#6045 is a backport of this pull request to Red 3.4.

@github-actions github-actions bot added the Category: Core - API - Config This is related to the `redbot.core.config` module and `redbot.core.drivers` package. label Apr 19, 2023
Jackenmen added a commit that referenced this pull request Apr 19, 2023
@Red-GitHubBot @Jackenmen
[3.4] Properly handle missing schemas/tables in PostgreSQL driver (#5855
20dfe8a 
) (#6045)

Co-authored-by: Jakub Kuczys <me@jacken.men>
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kowlin Kowlin Kowlin approved these changes

Assignees
No one assigned
Labels
Category: CI This is related to repository's CI configuration. Category: Core - API - Config This is related to the `redbot.core.config` module and `redbot.core.drivers` package. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. hacktoberfest-accepted Used to mark a PR as valid Hacktoberfest contribution. DO NOT REMOVE UNTIL END OF NOVEMBER (sic!)! Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Milestone
3.4.19
Development

Successfully merging this pull request may close these issues.

Red is spamming postgres with an invalid query
2 participants
@Jackenmen @Kowlin