feat: add database param to connect() for reusing session pool

[waiho-gumloop]

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Summary of Changes

Hello @waiho-gumloop, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant improvement to how database connections are managed by allowing multiple DBAPI connections to share a single Spanner session pool. By enabling the connect() function to accept a pre-existing Database object, it optimizes resource utilization and reduces the overhead associated with creating new session pools for each connection, aligning with the underlying Spanner client's architecture.

Highlights

• New database parameter in connect(): The connect() function now accepts an optional database parameter, allowing users to provide an existing Database object to facilitate session pool reuse.
• Session pool reuse optimization: When a Database object is supplied, the new connection will utilize its existing session pool, avoiding the creation of a new PingingPool and associated BatchCreateSessions RPC calls for each connection.
• Shared session pool management: Connections created with a shared database object will not own the session pool, ensuring that closing such a connection does not inadvertently destroy the shared resource.

Changelog

• google/cloud/spanner_dbapi/connection.py
  • Added a new optional database parameter to the connect function signature.
  • Included comprehensive documentation for the database parameter, explaining its purpose for sharing session pools.
  • Implemented conditional logic within connect to create a lightweight connection wrapper that uses the provided database's session pool if the database parameter is not None.
  • Set the _own_pool attribute to False for connections utilizing a shared pool to prevent its destruction upon connection closure.

Activity

• No specific activity (comments, reviews, or progress updates) has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a useful feature for reusing session pools by passing a Database object to connect(). However, the current implementation can lead to confusion because it silently ignores several other connection parameters when a database object is provided. I've added a specific comment with a suggestion to include validation checks to prevent potential misconfigurations. Additionally, this new functionality is not covered by tests. Please add unit tests to verify the new behavior, including the proposed validation logic.

[gemini-code-assist]

When a database object is provided, several other parameters to connect() are ignored because the connection is created directly from the database object. This can be confusing and lead to subtle bugs if a user provides these parameters expecting them to have an effect.

Since instance_id is a required argument, it's particularly problematic as it must be provided but is then not used to establish the connection (other than for a potential mismatch).

To improve the API's robustness and prevent misconfiguration, I recommend adding validation to ensure that when database is provided:

1. The required instance_id matches the one in the database object.
2. Other conflicting and redundant parameters are not provided.

    if database is not None:
        if instance_id != database._instance.instance_id:
            raise ValueError(
                f"The provided `instance_id` ('{instance_id}') does not match the "
                f"instance ID of the `database` object ('{database._instance.instance_id}')."
            )

        # These parameters are used to create a new client/database, so they conflict
        # with providing a pre-existing database object.
        conflicting_params = {
            "database_id": database_id,
            "project": project,
            "credentials": credentials,
            "pool": pool,
            "client": client,
            "database_role": database_role,
            "experimental_host": experimental_host,
        }
        provided_conflicting = [
            name for name, value in conflicting_params.items() if value is not None
        ]

        if provided_conflicting:
            raise ValueError(
                "When a `database` object is provided, the following parameters "
                f"are redundant and must not be set: {', '.join(provided_conflicting)}"
            )

        instance = database._instance
        conn = Connection(instance, database, **kwargs)
        conn._own_pool = False  # Don't destroy the shared pool on close
        return conn