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

Revert/Remove unnecessary database_name config #7049

Merged
merged 2 commits into from
Jul 3, 2020
Merged

Revert/Remove unnecessary database_name config #7049

merged 2 commits into from
Jul 3, 2020

Conversation

AlexandreYang
Copy link
Member

@AlexandreYang AlexandreYang commented Jul 2, 2020 •
edited
Loading

Partially Reverts #6835

What this PR does:

Motivation

When connecting to admin port of proxysql, providing database argument to pymysql.connect does not have any effect.

Queries like SELECT * FROM stats_mysql_global will continue to work for both admin and stats (users listed in stats_credentials in proxysql.cnf) users.

This is likely due to the fact that stats tables are available by default to both admin and stats users:

A user that connects to Admin with admin-stats_credentials credentials can only access this schema.

Additional details here: #6835 (comment)

cc @tabacco

@AlexandreYang AlexandreYang changed the title Revert "Allow proxysql checks to specify stats database name" Remove unnecessary database_name config Jul 2, 2020
@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #7049 into master will increase coverage by 4.01%.
The diff coverage is n/a.

Impacted Files Coverage Δ
proxysql/datadog_checks/proxysql/proxysql.py 95.45% <ø> (-0.07%) ⬇️
ibm_mq/tests/conftest.py
...kube_controller_manager/kube_controller_manager.py
couchbase/datadog_checks/couchbase/__init__.py
rethinkdb/tests/assertions.py
tomcat/tests/test_check.py
activemq_xml/tests/test_activemq_xml.py
mapr/datadog_checks/mapr/utils.py
ibm_mq/datadog_checks/ibm_mq/metrics.py
ibm_db2/datadog_checks/ibm_db2/__about__.py
... and 749 more

@AlexandreYang AlexandreYang marked this pull request as ready for review July 2, 2020 12:30
@AlexandreYang AlexandreYang requested review from a team as code owners July 2, 2020 12:30
@AlexandreYang AlexandreYang changed the title Remove unnecessary database_name config Revert/Remove unnecessary database_name config Jul 2, 2020
apigirl
apigirl approved these changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@apigirl apigirl left a comment

Choose a reason for hiding this comment

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

👍 from docs

@FlorianVeaux FlorianVeaux merged commit d6cd177 into master Jul 3, 2020
@FlorianVeaux FlorianVeaux deleted the revert-6835-support-proxysql-stats-user branch July 3, 2020 09:30
FlorianVeaux pushed a commit that referenced this pull request Jul 3, 2020
@AlexandreYang @FlorianVeaux
Revert/Remove unnecessary database_name config (#7049)
988b501 
* Revert "Allow proxysql checks to specify stats database name (#6835)"

This reverts commit 71fa47a.

* Keep removal of stats. prefix and tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apigirl apigirl apigirl approved these changes

@FlorianVeaux FlorianVeaux FlorianVeaux approved these changes

Assignees
No one assigned
Labels
documentation integration/proxysql
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexandreYang @FlorianVeaux @apigirl