Skip to content

Conversation

@Shivansh-007
Copy link
Member

@Shivansh-007 Shivansh-007 commented Aug 28, 2021

Closes #55

This PR adds an async PostgreSQL setup with the help of SQLAlchemy and alembic. Alembic
is a tool for keeping track of the database migration history and SQLAlchemy(with asyncpg) is a popular ORM tool. The following models have been added to the DB:

  1. Attachments
  2. Configurations
  3. Embeds
  4. Emojis
  5. Messages
  6. Tickets

Where the Embeds and Attachments are foreign keys to the Messages.id column.

TODO

  • Document migrations and database setup.
  • Add tests
  • Add CRUD functions for each model
  • Docker support (deffered)
  • Changelog this

This commit also cleans up redundant imports in `config.py` and fixes
some type hints.
This function is ran before the discord modmail bot is ran so that the
bot is not ran unnecessarily even if the DB is not responding. It
creates a local session which can be used throughout the bot and then
tests if the db is responding.

If the DB is not responding it would log the error with the exception
and then exit, it doesn't raise the exception as the the sqlalchemy
connection errors are usually really long and maybe "spammy", to reduce
the logs down it just logs down the error and then exists silently.
Since we are getting the embed json content directly via discord.py's
embed and not generating or touching it, we are safe and validation
shouldn't be needed.

I have made an extra commit here, in case we ever need it again in
future we won't need to rewrite it all over again.
Because it stores things not in the messages/tickets, and it's just use
to refer to the message the thread starts from in the ticket logging
channel.

https://discord.com/channels/798235512208490526/845824657306288158/881136355348992060
Assert statements are removed when running in optimized mode, but we
don't.
@Shivansh-007 Shivansh-007 added a: database Related to or causes database changes p: high High Priority s: WIP Waiting for author to address a review or respond to a comment t: feature Relating to the functionality of the application. labels Aug 28, 2021
@Shivansh-007 Shivansh-007 self-assigned this Aug 28, 2021
@Shivansh-007 Shivansh-007 marked this pull request as draft August 28, 2021 14:34
@onerandomusername
Copy link
Contributor

Removed Shivansh as he's busy, @bast0006 is hopefully going to finish this pr.

@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #65 (b2480f8) into main (0ed105f) will decrease coverage by 5.69%.
The diff coverage is 8.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
- Coverage   38.62%   32.92%   -5.70%     
==========================================
  Files          16       27      +11     
  Lines         769      902     +133     
  Branches      105      110       +5     
==========================================
  Hits          297      297              
- Misses        459      590     +131     
- Partials       13       15       +2     
Impacted Files Coverage Δ
modmail/crud/configurations.py 0.00% <0.00%> (ø)
modmail/crud/messages.py 0.00% <0.00%> (ø)
modmail/crud/threads.py 0.00% <0.00%> (ø)
modmail/models/__init__.py 0.00% <0.00%> (ø)
modmail/models/attachments.py 0.00% <0.00%> (ø)
modmail/models/base.py 0.00% <0.00%> (ø)
modmail/models/configurations.py 0.00% <0.00%> (ø)
modmail/models/embeds.py 0.00% <0.00%> (ø)
modmail/models/emojis.py 0.00% <0.00%> (ø)
modmail/models/messages.py 0.00% <0.00%> (ø)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ed105f...b2480f8. Read the comment docs.

@Shivansh-007 Shivansh-007 marked this pull request as ready for review September 12, 2021 07:39
@Shivansh-007 Shivansh-007 added s: needs review Ready for review and merge and removed s: WIP Waiting for author to address a review or respond to a comment labels Sep 12, 2021
@Shivansh-007
Copy link
Member Author

Closing this, as I would be rewriting it using tortoise ORM.

@Shivansh-007 Shivansh-007 deleted the feat/db-client branch October 22, 2021 09:20
@Shivansh-007 Shivansh-007 removed their assignment Oct 22, 2021
@Shivansh-007 Shivansh-007 mentioned this pull request Nov 1, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: database Related to or causes database changes p: high High Priority s: needs review Ready for review and merge t: feature Relating to the functionality of the application.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Main feature: database

4 participants