Skip to content

Conversation

@ptiurin
Copy link
Contributor

@ptiurin ptiurin commented Oct 15, 2025

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

@ptiurin ptiurin marked this pull request as ready for review October 20, 2025 15:51
@ptiurin ptiurin requested a review from a team as a code owner October 20, 2025 15:51
Copy link
Contributor

@goprean goprean left a 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



@fixture
def query_callback_with_remove_header(query_statistics: Dict[str, Any]) -> Callable:
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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}&param1=value1&param2=value2&param3=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"

@ptiurin ptiurin requested a review from goprean October 21, 2025 13:56
@sonarqubecloud
Copy link

@ptiurin ptiurin requested a review from goprean October 21, 2025 16:44
@ptiurin ptiurin merged commit 18c93ed into main Oct 22, 2025
17 checks passed
@ptiurin ptiurin deleted the feat-remove-parameters branch October 22, 2025 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants