-
Notifications
You must be signed in to change notification settings - Fork 11
feat(FIR-46457): Support remove parameters backend header #468
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
Conversation
goprean
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good if the decision is to go with the cursor approach
tests/unit/db_conftest.py
Outdated
|
|
||
|
|
||
| @fixture | ||
| def query_callback_with_remove_header(query_statistics: Dict[str, Any]) -> Callable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we name this query_callback_with_remove_headers_param1_and_param3. It took me while to understand why only param1 and param3 were removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added parameters here. Hopefully it's clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this test, I did not figured out why param1 and param3 were removed but not param2:
```
# Execute query that returns remove parameters header
httpx_mock.reset()
httpx_mock.add_callback(
query_callback_with_remove_header,
url=f"{set_query_url}¶m1=value1¶m2=value2¶m3=value3&output_format=JSON_Compact",
is_reusable=True,
)
cursor.execute("SELECT 1")
# Verify that param1 and param3 were removed, param2 remains
assert len(cursor._set_parameters) == 1
assert "param1" not in cursor._set_parameters
assert "param2" in cursor._set_parameters
assert "param3" not in cursor._set_parameters
assert cursor._set_parameters["param2"] == "value2"
|



Support new header that enables work with transactions. This allows transactions limited to a single cursor. Wider, DBApi-compatible implementation is in discussion.