Skip to content

Comments

fix: make DatabaseSessionService visible to API documentation (#4331)#4608

Open
Akshat8510 wants to merge 6 commits intogoogle:mainfrom
Akshat8510:fix/database-session-service-docs-4331
Open

fix: make DatabaseSessionService visible to API documentation (#4331)#4608
Akshat8510 wants to merge 6 commits intogoogle:mainfrom
Akshat8510:fix/database-session-service-docs-4331

Conversation

@Akshat8510
Copy link
Contributor

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
The DatabaseSessionService class was missing from the generated ADK API Reference. This occurred because the class was being lazily loaded via the __getattr__ method in google.adk.sessions. Static analysis tools and documentation generators (like Sphinx/autodoc) typically cannot discover attributes that are not present in the module's namespace at import time.

Solution:
Replaced the __getattr__ lazy-loading logic with a top-level try...except ImportError block. This ensures that the DatabaseSessionService symbol is explicitly present in the module's namespace, making it visible to documentation tools. This approach preserves the intended behavior of supporting DatabaseSessionService as an optional dependency (requiring SQLAlchemy) without causing crashes for users who do not have it installed.

Testing Plan

Unit Tests:

  • All existing unit tests pass locally.

Manual End-to-End (E2E) Tests:
Verified that the class is now correctly exported and visible in the module namespace by running a manual import check in the development environment:

python -c "from google.adk.sessions import DatabaseSessionService; print('Logic is perfect')"

Output: Logic is perfect

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works. (Manual verification of import visibility)
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.

…#4331)

Replaces lazy loading via __getattr__ with a top-level try-except import. This allows documentation generators to discover the class while maintaining support for optional dependencies.

Signed-off-by: Akshat Kumar <akshat230405@gmail.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Akshat8510, I'm Gemini Code Assist1! 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 addresses an issue where the DatabaseSessionService class was not appearing in the generated API documentation. The core change involves refactoring how this class is imported within its module, moving from a dynamic, lazy-loading approach to a static, top-level import wrapped in an error handling block. This ensures that documentation tools can discover the class while preserving the flexibility for users who may not have its optional dependencies installed.

Highlights

  • Documentation Visibility: The DatabaseSessionService class is now correctly exposed to API documentation generators by replacing a lazy-loading mechanism with a top-level import.
  • Import Mechanism Refactor: The __getattr__ method, previously used for lazily importing DatabaseSessionService, has been removed. It is replaced by a try...except ImportError block at the module's top level.
  • Optional Dependency Handling: The change maintains the optional nature of DatabaseSessionService (which requires SQLAlchemy) by catching ImportError, ensuring users without the dependency installed do not encounter issues.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/google/adk/sessions/init.py
    • Removed the __getattr__ method responsible for lazy-loading DatabaseSessionService.
    • Added a top-level try...except ImportError block to import DatabaseSessionService, making it visible to static analysis and documentation tools.
Activity
  • All existing unit tests passed locally.
  • Manual end-to-end testing confirmed the class is now correctly exported and visible via an import check.
  • The author completed a self-review and ensured all checklist items were addressed.
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.

@Akshat8510
Copy link
Contributor Author

@gemini-code-assist/gemini review

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Feb 24, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to make DatabaseSessionService visible to API documentation generators by replacing the __getattr__ lazy loading with a top-level try...except ImportError block. While this change successfully addresses the documentation issue, it introduces a critical runtime bug. When the optional sqlalchemy dependency is not installed, DatabaseSessionService is not defined, but it remains in __all__. This leads to an AttributeError on import * and a non-descriptive ImportError on direct import. I've suggested a fix to define a placeholder class in the except block to resolve this, ensuring the symbol is always present while providing a clear error message if used without the required dependencies.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to make DatabaseSessionService visible to API documentation tools by changing how it's imported. While the approach of using a top-level try...except block is correct for making the class discoverable, the current implementation has a couple of issues. Firstly, it can cause an AttributeError when using from google.adk.sessions import * if optional dependencies aren't installed, because DatabaseSessionService is listed in __all__ but won't be defined. Secondly, it removes the helpful error message that was previously raised when a user tried to use DatabaseSessionService without the necessary dependencies. I've suggested a change to define a placeholder class in the except block. This placeholder will raise an informative ImportError upon instantiation, which resolves both issues while keeping the class visible for documentation.

…ovides a helpful error message if instantiated without sqlalchemy installed.

Signed-off-by: Akshat Kumar <akshat230405@gmail.com>
@Akshat8510
Copy link
Contributor Author

@gemini-code-assist/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses an issue where DatabaseSessionService was not visible to API documentation generators by replacing the __getattr__ lazy loading with a try...except block. This is a good approach. I've added one suggestion to improve the placeholder class by making it inherit from BaseSessionService. This will improve type compatibility for consumers of the library, especially for static analysis and runtime type checks.

…ror handling for missing dependencies.

Signed-off-by: Akshat Kumar <akshat230405@gmail.com>
@Akshat8510
Copy link
Contributor Author

@gemini-code-assist/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses the visibility issue of DatabaseSessionService for documentation tools by replacing the __getattr__ lazy loading with a try...except ImportError block. This is a good approach. I've added one suggestion to improve the implementation of the placeholder class for better readability and maintainability.

Signed-off-by: Akshat Kumar <akshat230405@gmail.com>
@Akshat8510
Copy link
Contributor Author

@gemini-code-assist/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses the issue of DatabaseSessionService not being visible to API documentation generators by replacing the dynamic __getattr__ loading with a try...except ImportError block. This is a robust solution that ensures the class is discoverable by static analysis tools while preserving the optional dependency behavior. I have one suggestion to improve code consistency by using the @override decorator in the new placeholder class.

…older per bot feedback

Signed-off-by: Akshat Kumar <akshat230405@gmail.com>
@Akshat8510
Copy link
Contributor Author

@gemini-code-assist/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses the issue of DatabaseSessionService not being visible to API documentation and static analysis tools. By replacing the __getattr__ lazy-loading mechanism with a try...except ImportError block and a placeholder class, the DatabaseSessionService symbol is now explicitly present in the module's namespace. This approach maintains the optional dependency behavior while ensuring proper discoverability for documentation generation, which is a significant improvement for maintainability and developer experience.

@Akshat8510
Copy link
Contributor Author

/gemini summary

@gemini-code-assist
Copy link
Contributor

Summary of Changes

This pull request addresses a critical issue where the DatabaseSessionService class was not being included in the generated API documentation. The core change involves moving from a dynamic, lazy-loading import strategy to a more explicit try...except block. This ensures that static analysis tools and documentation generators can correctly identify and document the class, while still maintaining the optional nature of its dependencies and providing clear error messages to users if those dependencies are not met.

Highlights

  • Improved API Documentation Visibility: The DatabaseSessionService class is now explicitly imported, ensuring it is discoverable by documentation generation tools like Sphinx/autodoc, resolving a previous issue where it was omitted.
  • Refactored Optional Dependency Handling: The dynamic lazy-loading mechanism using __getattr__ for DatabaseSessionService has been replaced with a static try...except ImportError block. This allows the module to gracefully handle cases where sqlalchemy (an optional dependency) is not installed.
  • Robust Placeholder Class Implementation: A placeholder DatabaseSessionService class is now defined within the except ImportError block. This placeholder inherits from BaseSessionService, uses @override decorators, and raises informative ImportError messages upon instantiation or method calls if sqlalchemy is missing, ensuring type consistency and clear error feedback.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/google/adk/sessions/init.py
    • Removed the __getattr__ method that previously handled lazy loading of DatabaseSessionService.
    • Added a try...except ImportError block to import DatabaseSessionService directly.
    • Introduced a placeholder DatabaseSessionService class within the except block that inherits from BaseSessionService and raises ImportError if its methods are called without sqlalchemy installed.
    • Imported override from typing_extensions and applied it to overridden methods in the placeholder class.
Activity
  • An initial summary of changes was provided by gemini-code-assist[bot].
  • gemini-code-assist[bot] provided several review comments, initially highlighting issues with import * behavior and less helpful error messages after the initial change.
  • Further review comments from gemini-code-assist[bot] suggested improvements to the placeholder class, including inheriting from BaseSessionService, raising ImportError directly in methods, and using the @override decorator for clarity.
  • The author, Akshat8510, repeatedly requested reviews, indicating an iterative process of incorporating feedback.
  • The author confirmed all existing unit tests passed locally and performed manual end-to-end testing to verify the class's visibility and correct import behavior.

@Akshat8510
Copy link
Contributor Author

Hi @wuliang229, I’ve finalized #4331 and addressed all architectural feedback, replaced lazy-loading with a top-level try/except, added a proper placeholder inheriting from BaseSessionService for Mypy consistency, and applied the suggested refactors.

@Akshat8510
Copy link
Contributor Author

The gemini-code-assist bot marked it as a significant improvement with no further concerns. Thanks!

@gemini-code-assist
Copy link
Contributor

That's great to hear, @Akshat8510! I'm glad the feedback was helpful in refining the pull request. The changes look solid and effectively address the initial documentation visibility issue while maintaining robust optional dependency handling. Well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing API reference for DatabaseSessionService

2 participants