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.

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

Update ProxySQL check to use TLS context wrapper #8243

Merged
merged 15 commits into from
Jan 4, 2021
Merged

Conversation

yzhan289
Copy link
Contributor

@yzhan289 yzhan289 commented Dec 22, 2020

What does this PR do?

This PR does a few things:

  • Update the ProxySQL check to use SSL wrapper instead of own wrapper.
  • Separate test_proxysql.py into test_unit.py, test_integration.py, test_e2e for readability.
  • Add docs for how to use SSL.

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #8243 (20d46f4) into master (ffbc737) will increase coverage by 10.51%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
proxysql/datadog_checks/proxysql/ssl_utils.py 100.00% <ø> (+65.21%) ⬆️
proxysql/datadog_checks/proxysql/proxysql.py 98.57% <100.00%> (+3.11%) ⬆️
proxysql/tests/common.py 100.00% <100.00%> (ø)
proxysql/tests/conftest.py 100.00% <100.00%> (ø)
proxysql/tests/test_integration.py 100.00% <100.00%> (ø)
proxysql/tests/test_unit.py 100.00% <100.00%> (ø)
gitlab/datadog_checks/gitlab/__about__.py
lighttpd/datadog_checks/lighttpd/lighttpd.py
..._state/datadog_checks/kubernetes_state/__init__.py
gitlab/tests/common.py
... and 787 more

@yzhan289 yzhan289 changed the title Update ProxySQL to use SSL wrapper. Update ProxySQL check to use SSL context wrapper Dec 22, 2020
@DataDog DataDog deleted a comment from azure-pipelines bot Dec 22, 2020
@yzhan289
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@yzhan289 yzhan289 changed the title Update ProxySQL check to use SSL context wrapper Update ProxySQL check to use TLS context wrapper Dec 23, 2020
Copy link
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

Docs review: Minor formatting suggestion, and a request for an example in the readme.

proxysql/README.md Outdated Show resolved Hide resolved
@yzhan289 yzhan289 requested a review from kayayarai December 28, 2020 18:50
proxysql/README.md Outdated Show resolved Hide resolved
@yzhan289 yzhan289 requested a review from FlorianVeaux January 4, 2021 14:44
Copy link
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

docs 👍

@yzhan289 yzhan289 merged commit 1659b6c into master Jan 4, 2021
@yzhan289 yzhan289 deleted the az/proxysql-ssl branch January 4, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants