Skip to content

[WIP] Cluster mode example: factor out the database#182

Draft
gaborfeher wants to merge 2 commits intoa2aproject:mainfrom
gaborfeher:gaborfeher-db-taskstore
Draft

[WIP] Cluster mode example: factor out the database#182
gaborfeher wants to merge 2 commits intoa2aproject:mainfrom
gaborfeher:gaborfeher-db-taskstore

Conversation

@gaborfeher
Copy link

specific logic from taskstore.go into db_taskstore.go

specific logic from taskstore.go into db_taskstore.go
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @gaborfeher, 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 refactors the database interaction logic within the cluster mode example. The primary goal is to separate the database-specific implementation from the generic task storage mechanism. This is achieved by introducing a Database interface and a generic DBTaskStore that operates against this interface, along with a concrete MySQL implementation. This change significantly improves the modularity and extensibility of the task persistence layer, making it easier to support different database backends in the future.

Highlights

  • Database Abstraction: Introduced a new Database interface in utils/db_taskstore.go which defines the core persistence operations (Create, Update, Get) for tasks and events. This abstracts the database-specific implementation details.
  • Generic DBTaskStore: Created a new DBTaskStore in utils/db_taskstore.go that implements the a2asrv.TaskStore interface using the newly defined Database interface. This centralizes the database-agnostic task storage logic.
  • MySQL Implementation: Added mysql_db_adapter.go which provides a concrete implementation of the Database interface specifically for MySQL, handling SQL transactions, optimistic concurrency, and JSON (un)marshalling for task and event data.
  • Refactored TaskStore: The original taskstore.go has been significantly refactored. It no longer contains the direct database interaction logic but now acts as a factory, creating and utilizing the MySQLDBAdapter and the generic DBTaskStore.
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.

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 refactors the database-specific logic out of taskstore.go by introducing a Database interface and a MySQLDBAdapter implementation. This is a great improvement for modularity and prepares the ground for supporting other database backends. The code is well-structured and the abstraction is clean. I have a few suggestions to remove an unused parameter from the Update method signature across the new interface and implementation, which will make the code even clearer.

}

// Update persists an updated task and event.
func (a *MySQLDBAdapter) Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64, protocolVersion a2a.ProtocolVersion) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The protocolVersion parameter is not used within this function. To align with the interface change and improve clarity, it should be removed from the function signature.

Suggested change
func (a *MySQLDBAdapter) Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64, protocolVersion a2a.ProtocolVersion) (bool, error) {
func (a *MySQLDBAdapter) Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64) (bool, error) {

Create(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, protocolVersion a2a.ProtocolVersion) error
// Update persists an updated task and event.
// It returns true if the operation was successful (or false if optimistic locking failed).
Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64, protocolVersion a2a.ProtocolVersion) (bool, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The protocolVersion parameter is unused in implementations of this Update method (e.g., MySQLDBAdapter). Since the protocol version of a task is not expected to change after creation, this parameter should be removed from the Update method signature to improve code clarity. This will require updating the method signature here, in the MySQLDBAdapter.Update implementation, and the call to s.db.Update in DBTaskStore.Save.

Suggested change
Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64, protocolVersion a2a.ProtocolVersion) (bool, error)
Update(ctx context.Context, task *a2a.Task, event a2a.Event, newVersion int64, prevVersion int64) (bool, error)

return a2a.TaskVersionMissing, err
}
} else {
success, err := s.db.Update(ctx, task, event, newVersion, int64(prevVersion), s.version)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

To align with the suggested change of removing the unused protocolVersion parameter from the db.Update method, this call should be updated accordingly.

Suggested change
success, err := s.db.Update(ctx, task, event, newVersion, int64(prevVersion), s.version)
success, err := s.db.Update(ctx, task, event, newVersion, int64(prevVersion))

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

Comments