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

Fix adapter failures due to string formatting issues #4305

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

nathaniel-may
Copy link
Contributor

@nathaniel-may nathaniel-may commented Nov 18, 2021

Description

This is an alternative solution to dbt-labs/dbt-redshift#43. Prior to this change, log messages passed to the AdapterLogger that had brackets in them would trigger a KeyError when we tried to format with an empty set of arguments. This solution is preferred to dbt-labs/dbt-redshift#43 because it fixes an existing interface problem.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Nice catch!

Confirmed this fixed test_redshift_column_quotes (one of the nightly failures) when I ran it locally with this branch

Comment on lines +24 to +47
def test_formatting(self):
logger = AdapterLogger("dbt_tests")
# tests that it doesn't throw
logger.debug("hello {}", 'world')

# enters lower in the call stack to test that it formats correctly
event = AdapterEventDebug(name="dbt_tests", base_msg="hello {}", args=('world',))
self.assertTrue("hello world" in event.message())

# tests that it doesn't throw
logger.debug("1 2 {}", 3)

# enters lower in the call stack to test that it formats correctly
event = AdapterEventDebug(name="dbt_tests", base_msg="1 2 {}", args=(3,))
self.assertTrue("1 2 3" in event.message())

# tests that it doesn't throw
logger.debug("boop{x}boop")

# enters lower in the call stack to test that it formats correctly
# in this case it's that we didn't attempt to replace anything since there
# were no args passed after the initial message
event = AdapterEventDebug(name="dbt_tests", base_msg="boop{x}boop", args=())
self.assertTrue("boop{x}boop" in event.message())
Copy link
Member

Choose a reason for hiding this comment

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

👍

@nathaniel-may nathaniel-may merged commit b4793b4 into main Nov 18, 2021
@nathaniel-may nathaniel-may deleted the fix-log-string-formatting branch November 18, 2021 17:54
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
fix adapter failures due to string formatting issues

automatic commit by git-black, original commits:
  b4793b4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants