feat: Add Go SDK for the Jules API #3
Conversation
This commit introduces a new Go SDK for interacting with the Jules API. The SDK is located in the `go_sdk` directory and is separate from the existing Python SDK. The Go SDK includes: - A base HTTP client with support for API key authentication, retries with exponential backoff, and configurable timeouts. - Data models for all API resources, including Sessions, Activities, and Sources. - Services for each API resource, providing methods for creating, getting, and listing resources. - A main `JulesClient` that provides a single entry point for all API services. - A usage example in the `examples/go_simple_test` directory to demonstrate how to use the SDK.
Summary of ChangesHello @sashimikun, 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 delivers a new, fully-featured Go SDK for the Jules API. It aims to empower Go developers with a robust and idiomatic way to integrate with the Jules platform, covering key functionalities like session management, activity tracking, and source interaction. The SDK is designed for reliability and ease of use, incorporating best practices for API client development. Highlights
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Go SDK for the Jules API, which is a great addition. The overall structure is sound, with a clear separation of concerns between the client, services, and models. I've identified a few critical and high-severity issues, particularly around the HTTP client's retry logic and context handling in long-running operations. There are also some medium-severity suggestions to improve robustness, error handling, and adherence to Go conventions. Addressing these points will significantly improve the quality and usability of the SDK.
| var err error | ||
|
|
||
| for i := 0; i < c.maxRetries; i++ { | ||
| resp, err = c.httpClient.Do(req) |
There was a problem hiding this comment.
The current retry logic reuses the same http.Request object for each attempt. When a request has a body (e.g., a POST request), the body is an io.Reader that gets consumed on the first attempt. Subsequent retries will then send an empty body, which is a critical bug. To fix this, you should clone the request for each retry attempt. req.Clone(req.Context()) will create a new request with a fresh body stream, provided req.GetBody is set, which http.NewRequest does automatically for *bytes.Buffer.
clonedReq := req.Clone(req.Context())
resp, err = c.httpClient.Do(clonedReq)| if err == nil && resp.StatusCode < 500 { | ||
| break // Success or non-retriable error | ||
| } | ||
| time.Sleep(time.Duration(i*i) * time.Second) // Exponential backoff |
There was a problem hiding this comment.
The comment states this is an "Exponential backoff", but the implementation time.Duration(i*i) * time.Second is quadratic. Furthermore, the first retry happens with a 0-second delay, which is not ideal. You should implement a proper exponential backoff, for example by using bit-shifting.
time.Sleep(time.Duration(1<<uint(i)) * time.Second) // Exponential backoff| func (s *SessionsService) WaitForCompletion(sessionID string) (*Session, error) { | ||
| timeout := time.After(DefaultTimeout) | ||
| ticker := time.NewTicker(DefaultPollInterval) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-timeout: | ||
| return nil, fmt.Errorf("timed out waiting for session %s to complete", sessionID) | ||
| case <-ticker.C: | ||
| session, err := s.Get(sessionID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| switch session.State { | ||
| case Completed: | ||
| return session, nil | ||
| case Failed: | ||
| return session, fmt.Errorf("session %s failed", sessionID) | ||
| } | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The WaitForCompletion function is a long-running, blocking operation. In idiomatic Go, such functions should accept a context.Context as the first argument. This allows the caller to control cancellation and timeouts, making the SDK more robust and easier to integrate. The hardcoded DefaultTimeout should be removed in favor of the context's deadline.
func (s *SessionsService) WaitForCompletion(ctx context.Context, sessionID string) (*Session, error) {
ticker := time.NewTicker(DefaultPollInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-ticker.C:
session, err := s.Get(sessionID)
if err != nil {
return nil, err
}
switch session.State {
case Completed:
return session, nil
case Failed:
return session, fmt.Errorf("session %s failed", sessionID)
}
}
}
}| } | ||
|
|
||
| func (c *Client) newRequest(method, path string, body interface{}) (*http.Request, error) { | ||
| url := fmt.Sprintf("%s/%s", c.baseURL, path) |
There was a problem hiding this comment.
Using fmt.Sprintf to join URL path segments can be fragile as it doesn't handle edge cases like trailing or leading slashes gracefully. A more robust approach is to use net/url.JoinPath (available in Go 1.19+), which is designed for this purpose. You will need to import net/url.
| if resp.StatusCode >= 400 { | ||
| return resp, fmt.Errorf("API error: %s", resp.Status) | ||
| } |
There was a problem hiding this comment.
The error message for non-successful status codes is very generic. To make debugging easier for users of the SDK, you should read the response body and include it in the error message, as APIs often provide detailed error information in the response payload.
if resp.StatusCode >= 400 {
body, _ := io.ReadAll(resp.Body)
return resp, fmt.Errorf("API error: %s, response: %s", resp.Status, string(body))
}| func (c *Client) get(path string, v interface{}) (*http.Response, error) { | ||
| req, err := c.newRequest("GET", path, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return c.do(req, v) | ||
| } |
There was a problem hiding this comment.
The code formatting in this function and others within the file is inconsistent and doesn't follow standard Go style (e.g., indentation is incorrect). This makes the code harder to read. Please run gofmt -w . on the project to automatically fix these formatting issues.
func (c *Client) get(path string, v interface{}) (*http.Response, error) {
req, err := c.newRequest("GET", path, nil)
if err != nil {
return nil, err
}
return c.do(req, v)
}| type Source struct { | ||
| Name string `json:"name"` | ||
| ID string `json:"id"` | ||
| GithubRepo *GitHubRepo `json:"githubRepo,omitempty"` |
There was a problem hiding this comment.
In Go, it's conventional to capitalize common initialisms like 'ID', 'URL', and 'GitHub'. The field name GithubRepo should be GitHubRepo to align with this convention and for consistency with the type name GitHubRepo.
| GithubRepo *GitHubRepo `json:"githubRepo,omitempty"` | |
| GitHubRepo *GitHubRepo `json:"githubRepo,omitempty"` |
|
|
||
| type SourceContext struct { | ||
| Source string `json:"source"` | ||
| GithubRepoContext *GitHubRepoContext `json:"githubRepoContext,omitempty"` |
There was a problem hiding this comment.
In Go, it's conventional to capitalize common initialisms like 'ID', 'URL', and 'GitHub'. The field name GithubRepoContext should be GitHubRepoContext to align with this convention and for consistency with the type name GitHubRepoContext.
| GithubRepoContext *GitHubRepoContext `json:"githubRepoContext,omitempty"` | |
| GitHubRepoContext *GitHubRepoContext `json:"githubRepoContext,omitempty"` |
1 participant
This commit introduces a new Go SDK for interacting with the Jules API. The SDK is located in the
go_sdkdirectory and is separate from the existing Python SDK.The Go SDK includes:
JulesClientthat provides a single entry point for all API services.examples/go_simple_testdirectory to demonstrate how to use the SDK.