Skip to content

add support for ParameterStatus response interceptors#7

Merged
bentsku merged 1 commit into
masterfrom
add-support-response-interceptor
Oct 18, 2023
Merged

add support for ParameterStatus response interceptors#7
bentsku merged 1 commit into
masterfrom
add-support-response-interceptor

Conversation

@bentsku
Copy link
Copy Markdown

@bentsku bentsku commented Oct 17, 2023

Add support for response interceptors, needed for upgrading flake8 and black via a transitive dependency to packaging>22.
This allows an interceptor in the shape of the following:

class ResponseParameterStatusInterceptor:
    def rewrite_parameter(key: str, value: str, context: dict=None) -> tuple[str, str]: ...

To intercept and modify the key and value of the ParameterStatus (see https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-PARAMETERSTATUS).

This is because Postgres would return the following for the server version:

b'S' b'application_name\x00_pytest.python\x00'
b'S' b'client_encoding\x00UTF8\x00'
b'S' b'DateStyle\x00ISO, MDY\x00'
b'S' b'integer_datetimes\x00on\x00'
b'S' b'IntervalStyle\x00postgres\x00'
b'S' b'is_superuser\x00on\x00'
b'S' b'server_encoding\x00UTF8\x00'
b'S' b'server_version\x0011.21 (Debian 11.21-1.pgdg110+1)\x00'
b'S' b'session_authorization\x00test1\x00'
b'S' b'standard_conforming_strings\x00on\x00'
b'S' b'TimeZone\x00Etc/UTC\x00'

As we can see, the server_version 11.21 (Debian 11.21-1.pgdg110+1) is quite complicated, and the redshift_connector expect a version to be strict semantic versioning (using packaging>22), and will fail while trying to parse this.

We will add an interceptor modifying the server_version to be 11.21 only.

\cc @alexrashed

@bentsku bentsku added the enhancement New feature or request label Oct 17, 2023
@bentsku bentsku self-assigned this Oct 17, 2023
@bentsku bentsku requested review from steffyP and whummer October 17, 2023 22:42
Copy link
Copy Markdown
Member

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

Amazing work, @bentsku! 🎉
Having ResponseInterceptors enables various use cases 🙂

The changes look good to me 👍

@bentsku bentsku merged commit 55232b4 into master Oct 18, 2023
@bentsku bentsku deleted the add-support-response-interceptor branch October 18, 2023 09:49
@bentsku
Copy link
Copy Markdown
Author

bentsku commented Oct 18, 2023

Having ResponseInterceptors enables various use cases 🙂

Yes, exactly! If we need to intercept another kind of response, we might need to still extend this, but we can follow the same pattern now 😄

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.

2 participants