Skip to content

support for async call to repo sync by adding new method#76

Open
3choBoomer wants to merge 4 commits into
drone:masterfrom
3choBoomer:Nathan-Cooke/Support-Async-call-to-Repo-Sync-v2
Open

support for async call to repo sync by adding new method#76
3choBoomer wants to merge 4 commits into
drone:masterfrom
3choBoomer:Nathan-Cooke/Support-Async-call-to-Repo-Sync-v2

Conversation

@3choBoomer

Copy link
Copy Markdown

Summary

Adds a new RepoListSyncAsync() method to the Client interface and its implementation. It posts to the same endpoint as RepoListSync but with async=true appended to the URL, causing the server to trigger the sync and return immediately.

This is one of two proposed approaches to resolve #74, which has been open since 2024. See #XX for an alternative implementation that extends RepoListSync with a functional options pattern instead.

Changes

  • Adds RepoListSyncAsync() ([]*Repo, error) to the Client interface
  • Adds the corresponding (c *client) RepoListSyncAsync() implementation in client.go
  • RepoListSync is untouched

Usage

// Existing — blocking sync, unchanged
repos, err := client.RepoListSync()

// New — non-blocking async sync
repos, err := client.RepoListSyncAsync()

Notes

This approach keeps each method's intent explicit and self-documenting — the function name alone communicates the async behaviour without requiring callers to know about option constructors.

Alternatives

See #75 for an alternative that extends RepoListSync with a functional options pattern instead. That approach keeps the Client interface surface smaller and is more extensible if additional query parameters are needed in the future, at the cost of a slightly less obvious call site.

@CLAassistant

CLAassistant commented Mar 17, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Ompragash Ompragash left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks for the PR! The approach of adding a dedicated method is clean and non-breaking. nice work.

Comment thread drone/client.go Outdated
Comment thread drone/interface.go Outdated
@ebtasam-faridy

Copy link
Copy Markdown

Hi @3choBoomer 3choBoomer,
Would it be possible to sync briefly before we move forward with this PR? I have a couple of suggestions:

  1. The async server path returns 204 No Content (empty body), but the current client flow still tries to JSON-decode the response body. That can surface as an EOF error.
  2. It would help to add focused tests for this behaviour:
    a client-side test for async sync behaviour
    a test that explicitly validates handling of empty/204 responses from drone/drone

@3choBoomer

Copy link
Copy Markdown
Author

@Ompragash @ebtasam-faridy I am currently traveling with family and I will circle back with you guys when I return to work on Thursday.

3choBoomer and others added 3 commits March 30, 2026 13:54
Co-authored-by: OP (oppenheimer) <21008429+Ompragash@users.noreply.github.com>
Co-authored-by: OP (oppenheimer) <21008429+Ompragash@users.noreply.github.com>
@3choBoomer 3choBoomer requested a review from Ompragash March 30, 2026 19:01
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.

4 participants