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(dbapi): avoid running % format with no query parameters #348

Merged
merged 4 commits into from Oct 29, 2020

Conversation

@HemangChothani
Copy link
Contributor

@HemangChothani HemangChothani commented Oct 28, 2020

Fixes #346

@HemangChothani HemangChothani requested a review from tswast Oct 28, 2020
@HemangChothani HemangChothani requested a review from as a code owner Oct 28, 2020
@google-cla google-cla bot added the cla: yes label Oct 28, 2020
def test__format_operation_w_empty_dict(self):
from google.cloud.bigquery.dbapi import cursor

formatted_operation = cursor._format_operation("SELECT 1", {},)
Copy link
Contributor

@tswast tswast Oct 28, 2020

Choose a reason for hiding this comment

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

Won't the linter complain about the trailing comma here?

Loading

tswast
tswast approved these changes Oct 28, 2020
from google.cloud.bigquery.dbapi import cursor

formatted_operation = cursor._format_operation("SELECT 1", {})
self.assertEqual(formatted_operation, "SELECT 1")
Copy link
Contributor

@tswast tswast Oct 28, 2020

Choose a reason for hiding this comment

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

Actually, let's put a % sign in this string to catch the reported error

Suggested change
self.assertEqual(formatted_operation, "SELECT 1")
self.assertEqual(formatted_operation, "SELECT '%f'")

Loading

tswast
tswast approved these changes Oct 29, 2020
@tswast tswast changed the title fix: aviod running %format when no query params fix(dbapi): avoid running % format with no query parameters Oct 29, 2020
@tswast tswast merged commit 5dd1a5e into googleapis:master Oct 29, 2020
10 checks passed
Loading
@tswast
Copy link
Contributor

@tswast tswast commented Oct 29, 2020

Thanks for the quick fix!

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants