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

Add OAuth authentication option and use new connection on check run #7703

Merged
merged 7 commits into from
Oct 7, 2020

Conversation

ChristineTChen
Copy link
Contributor

What does this PR do?

This PR adds OAuth as per beta user feedback.

This PR also fixes connection issue because the snowflake python connector's connection must be explicitly closed. see snowflakedb/snowflake-connector-python/issues/42

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 Oct 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@22dd4c4). Click here to learn what that means.
The diff coverage is 97.82%.

Impacted Files Coverage Δ
snowflake/datadog_checks/snowflake/check.py 93.65% <85.71%> (ø)
snowflake/datadog_checks/snowflake/config.py 89.28% <100.00%> (ø)
snowflake/tests/test_unit.py 96.77% <100.00%> (ø)

AlexandreYang
AlexandreYang previously approved these changes Oct 6, 2020
def test_default_authentication(instance):
# Test default auth
check = SnowflakeCheck(CHECK_NAME, {}, [instance])
assert check.config.authenticator == 'snowflake'
Copy link
Member

Choose a reason for hiding this comment

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

(nit) I would favour testing that sf.connect() is receiving the correct authenticator param using a mock.assert_called_with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@ChristineTChen ChristineTChen merged commit 3797e8a into master Oct 7, 2020
@ChristineTChen ChristineTChen deleted the cc/snowflake-oauth branch October 7, 2020 15:19
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.

None yet

2 participants