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

user: Add mysql user db implementation. #1419

Merged
merged 20 commits into from
Jun 17, 2021
Merged

user: Add mysql user db implementation. #1419

merged 20 commits into from
Jun 17, 2021

Conversation

amass01
Copy link
Member

@amass01 amass01 commented May 22, 2021

Closes #1392.

@amass01 amass01 force-pushed the sqluser branch 7 times, most recently from c06a1a9 to bcd5d46 Compare May 30, 2021 18:12
lukebp
lukebp requested changes May 30, 2021
View reviewed changes
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

This looks correct from a high level. I'll give it a more thorough review once its completed.

politeiawww/config.go Outdated Show resolved Hide resolved
politeiawww/config.go Outdated Show resolved Hide resolved
politeiawww/www.go Outdated Show resolved Hide resolved
scripts/userdb/mysqlsetup.sh Outdated Show resolved Hide resolved
@amass01 amass01 force-pushed the sqluser branch 2 times, most recently from 5addcc7 to 8ce4817 Compare June 1, 2021 02:12
@amass01 amass01 changed the title [wip] user: Add mysql user db implementation. user: Add mysql user db implementation. Jun 1, 2021
@amass01 amass01 marked this pull request as ready for review June 1, 2021 02:13
@amass01 amass01 force-pushed the sqluser branch 2 times, most recently from 32c81dd to f7dcd68 Compare June 2, 2021 17:49
lukebp
lukebp requested changes Jun 2, 2021
View reviewed changes
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

Still waiting to test this PR. Let's get the comments addressed and I'll give it a round of testing.

politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Show resolved Hide resolved
scripts/cmssetup.sh Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Show resolved Hide resolved
politeiawww/user/mysqldb/mysqldb.go Outdated Show resolved Hide resolved
politeiawww/user/mysqldb/mysqldb.go Outdated Show resolved Hide resolved
politeiawww/user/mysqldb/mysqldb.go Outdated Show resolved Hide resolved
lukebp
lukebp requested changes Jun 3, 2021
View reviewed changes
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/user/cockroachdb/log.go Outdated Show resolved Hide resolved
politeiawww/user/mysqldb/log.go Outdated Show resolved Hide resolved
@amass01 amass01 force-pushed the sqluser branch 2 times, most recently from 3e74118 to ac0782d Compare June 3, 2021 18:21
lukebp
lukebp requested changes Jun 4, 2021
View reviewed changes
politeiawww/config.go Outdated Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Outdated Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Outdated Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Outdated Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Outdated Show resolved Hide resolved
@amass01
Copy link
Member Author

amass01 commented Jun 5, 2021 •
edited
Loading

Looking good after some testing, just need to add TestPaywallAddressIndex to mysql_test.go.

Update: Done.

lukebp
lukebp requested changes Jun 13, 2021
View reviewed changes
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

I tested the setup process and migrating from cockroachdb to mysql. I wasn't able to test the mysql implementation due to a bug on startup.

politeiawww/config.go Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
scripts/userdb/mysqlsetup.sh Outdated Show resolved Hide resolved
politeiawww/user/mysql/mysql.go Show resolved Hide resolved
@amass01 amass01 force-pushed the sqluser branch 2 times, most recently from 1dff51c to 57b5e71 Compare June 15, 2021 02:24
@amass01
Add MySQL setup walkthrough to README
16f8631 
+ move scripts from root dir to ./politeiawww
lukebp
lukebp requested changes Jun 16, 2021
View reviewed changes
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

Tested the MySQL userdb and everything appears to be working as expected from a functionality standpoint. I've left a couple comments about a few areas that need to be improved or cleaned up.

Do not force push changes.

politeiawww/README.md Outdated Show resolved Hide resolved
politeiawww/README.md Show resolved Hide resolved
politeiawww/README.md Show resolved Hide resolved
politeiawww/config.go Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/config.go Show resolved Hide resolved
politeiawww/cmd/politeiawww_dbutil/politeiawww_dbutil.go Outdated Show resolved Hide resolved
politeiawww/README.md Show resolved Hide resolved
lukebp
lukebp requested changes Jun 17, 2021
View reviewed changes
politeiawww/README.md Outdated Show resolved Hide resolved
lukebp
lukebp approved these changes Jun 17, 2021
View reviewed changes
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

tACK

@lukebp lukebp merged commit 0b1982a into decred:master Jun 17, 2021
@amass01 amass01 deleted the sqluser branch June 22, 2021 12:12
@xaur xaur mentioned this pull request Jul 6, 2021
Closed
55 tasks
This was referenced Jul 9, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukebp lukebp lukebp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

user: Add mysql user db implementation.
2 participants
@amass01 @lukebp