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

[Audio] Change database backend #3193

Closed
wants to merge 25 commits into from
Closed

[Audio] Change database backend #3193

wants to merge 25 commits into from

Conversation

Drapersniper
Copy link
Contributor

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Removed trhe Database dependency and change it over to apsw, + update SQL statements and make them a little bit better

Drapersniper and others added 4 commits December 16, 2019 12:33
@Drapersniper
Initial commit
fc2853f
@Drapersniper
Workinnng
97f3b97 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
Improve SQL Statements + migrate from SQL Alchemy + Databases to APSW
9e07d8e 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
apsw tested and working
81f2e66 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
chose
508516a 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
mikeshardmind
mikeshardmind suggested changes Dec 18, 2019
View reviewed changes
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

Choose a reason for hiding this comment

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

This isn't a full review, just some things which I noticed at a relatively brief glance.

redbot/cogs/audio/apis.py Outdated Show resolved Hide resolved
redbot/cogs/audio/apis.py Outdated Show resolved Hide resolved
redbot/cogs/audio/apis.py Outdated Show resolved Hide resolved
Drapersniper and others added 7 commits December 18, 2019 11:32
@Drapersniper
Lets not touch the deps since #3192 is already adding them
ef38587
@Drapersniper
chore
a8d25ca
@Drapersniper
*sigh* Black
df72847
@Drapersniper
Update usage of last_fetched and last_updated to be Ints... However c…
ad866d6 
…olumn migration still pending
@Drapersniper
Delete 3195.misc.1.rst
030f7c8 
Wrong PR
@Drapersniper
Thanks Sinbad
6e8dcfb 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
Log Errors in init ...
9edda5c 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
mikeshardmind
mikeshardmind suggested changes Dec 20, 2019
View reviewed changes
redbot/cogs/audio/apis.py Outdated Show resolved Hide resolved
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Dec 20, 2019
Drapersniper and others added 4 commits December 20, 2019 08:24
@Drapersniper
Review sinbads latest comment ..
5ab3bb9 
ToDo.. Nuke existing playlist - check version and set version

Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
Merge branch 'V3/develop' of https://github.com/Cog-Creators/Red-Disc…
ff28ad8 
…ordBot into apsw
@Drapersniper
migrate the DB schema to v3 (to keep in line with the schema visionin…
70aa2a0 
…g of Config
@Drapersniper
*sigh* black
b1fdfa3
@mikeshardmind mikeshardmind self-assigned this Dec 20, 2019
@Drapersniper
Copy link
Contributor Author

Do we want the cache tables AND Playlist table to be in the same Database?
This PR uses "cache.db" but #3195 uses "Audio.db"... I'm all for making the cache tables into "Audio.db". WDYT @mikeshardmind. If the answer is yes should i reuse the connection/cursor or should i keep them separate

@mikeshardmind
Copy link
Contributor

Same DB, singular connection. Multiple cursors is fine.

@Drapersniper
#automagically
5c7bf19 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper
FINAFUCKINGLY
d70a292 
Signed-off-by: Drapersniper <27962761+drapersniper@users.noreply.github.com>
@Drapersniper Drapersniper mentioned this pull request Dec 21, 2019
Closed
@Drapersniper
Copy link
Contributor Author

Do not merge this one ... Merge #3205

@Flame442 Flame442 added the Type: Enhancement Something meant to enhance existing Red features. label Dec 22, 2019
@Drapersniper
Copy link
Contributor Author

Will setup a CPR for all of this so itsd easier to follow ... less likely to fuck up)

@Drapersniper Drapersniper deleted the apsw branch December 22, 2019 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikeshardmind mikeshardmind mikeshardmind requested changes

@Twentysix26 Twentysix26 Awaiting requested review from Twentysix26

Assignees

@mikeshardmind mikeshardmind

Labels
Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Drapersniper @mikeshardmind @Flame442