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

Added client encoding to adapted SQL parameters. #159

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

osvill
Copy link

@osvill osvill commented Mar 13, 2023

Fix & test for #158 .

@osvill osvill requested a review from palewire as a code owner March 13, 2023 15:41
@palewire
Copy link
Owner

This is an awesome start. Would it be possible to run a test that raises the encoding error so we can verify the reverse circumstance here?

…cute_sql. It now raises ValueError if client_encoding does not match the database encoding.
@osvill
Copy link
Author

osvill commented Mar 13, 2023

It is hard to catch the error as a specific encoding error. I ended up adding a keyword argument client_encoding and raising a ValueError if client_encoding does not match the database encoding (2nd commit). What do you think?

@osvill
Copy link
Author

osvill commented Mar 14, 2023

I ran some more tests and got errors for adapted parameters that do not have the encoding attribute. Therefore I added the hasattr check and the test filtering by a number in the 3rd commit.

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.

2 participants