Skip to content

Updated XRaySessionMaker constructor to follow SQLAlchemy's implementation #454

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kuredev
Copy link

@kuredev kuredev commented Mar 5, 2025

Description of changes:

I tested with the following version:

  • SQLAlchemy 2.0.38
  • aws-xray-sdk 2.14.0
  • Python 3.12.2
from aws_xray_sdk.ext.sqlalchemy.query import XRaySessionMaker

SessionLocal = XRaySessionMaker(bind=engine)

Running the above code results in the following error:

TypeError: sessionmaker.init() takes from 1 to 2 positional arguments but 7 were given

The XRaySessionMaker constructor acts as a wrapper around sqlalchemy.orm.session.sessionmaker's constructor.
However, the sessionmaker constructor expects keyword-only arguments, while XRaySessionMaker is calling it with positional arguments, resulting in an error

https://github.com/aws/aws-xray-sdk-python/blob/2.14.0/aws_xray_sdk/ext/sqlalchemy/query.py#L24-L25

        super().__init__(bind, class_, autoflush, autocommit, expire_on_commit,
                         info, **kw)

https://github.com/zzzeek/sqlalchemy/blob/rel_2_0_38/lib/sqlalchemy/orm/session.py#L5042-L5050

    def __init__(
        self: "sessionmaker[Session]",
        bind: Optional[_SessionBind] = ...,
        *,
        autoflush: bool = ...,
        expire_on_commit: bool = ...,
        info: Optional[_InfoType] = ...,
        **kw: Any,
    ): ...

Accordingly, I updated the call to use keyword arguments.
I have verified that tracing works correctly after applying this fix.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kuredev kuredev changed the title Change to keyword arguments to comply with SQLAlchemy specification u… Updated XRaySessionMaker constructor to follow SQLAlchemy's implementation Mar 5, 2025
@kuredev kuredev marked this pull request as ready for review March 5, 2025 15:35
@kuredev kuredev requested a review from a team as a code owner March 5, 2025 15:35
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.

1 participant