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

Implemented Connection#setStatementTimeout #200

Merged
merged 7 commits into from
Jan 22, 2024

Conversation

genzarchitect
Copy link
Contributor

Implemented Connection#setStatementTimeout function kindly check and approve :)

@jchrys
Copy link
Collaborator

jchrys commented Jan 15, 2024

Would it be possible for you to consider adding an integration test related to timeout scenarios?
This would help ensure the robustness of the code and provide better coverage.

@genzarchitect
Copy link
Contributor Author

Actually i am not more into testing .
Kindly Review and merge my PR. :)

@jchrys
Copy link
Collaborator

jchrys commented Jan 18, 2024

Actually i am not more into testing . Kindly Review and merge my PR. :)

We need test cases before merge this. Let me add some tests.

@jchrys
Copy link
Collaborator

jchrys commented Jan 18, 2024

PTAL @mirromutth @JohnNiang

@jchrys jchrys added the enhancement New feature or request label Jan 18, 2024
@jchrys jchrys force-pushed the trunk branch 3 times, most recently from 77d7600 to dc2dd93 Compare January 18, 2024 22:21
@jchrys jchrys self-assigned this Jan 18, 2024
@mirromutth
Copy link
Contributor

@jchrys That exception should be R2dbcNonTransientResourceException, because it is executing a statement which to set a timeout, not a query timed out.

It is caused by server version, so this failure is permanent unless user upgrade their database.

@jchrys
Copy link
Collaborator

jchrys commented Jan 19, 2024

@jchrys That exception should be R2dbcNonTransientResourceException, because it is executing a statement which to set a timeout, not a query timed out.

It is caused by server version, so this failure is permanent unless user upgrade their database.

Thanks for correction! Let me fix it :D

Motivation:
Previously, the `Connection#setStatementTimeout` method did not perform any operation (NO-OP).

Modification:
Successfully implemented the functionality for `Connection#setStatementTimeout`.

Result:
The `Connection#setStatementTimeout` method is now fully operational and functional.
Resolves asyncer-io#193

Co-authored-by: jchrys <[email protected]>
@jchrys
Copy link
Collaborator

jchrys commented Jan 19, 2024

PTAL @mirromutth

Copy link
Contributor

@mirromutth mirromutth left a comment

Choose a reason for hiding this comment

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

LGTM

@jchrys jchrys merged commit b6f367f into asyncer-io:trunk Jan 22, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants