refactor: deployment API & model for connection pattern & format code#160
Merged
pan93412 merged 2 commits intozeabur:mainfrom Jan 28, 2026
Merged
Conversation
Renamed deploymrnt.go to deployment.go and updated the deployment API to use DeploymentConnection and DeploymentEdge for GraphQL connection pattern. Adjusted related interface, model, and usage throughout the codebase. Also standardized struct definitions and improved code formatting in multiple files.
WalkthroughMostly formatting and import/layout cleanup across many files; plus functional changes: deployment pagination (edges connection and perPage), added Deployment timestamps (StartedAt, FinishedAt), new SHA-256 multipart upload flow for service uploads, and service lookup-by-name logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ServiceAPI as Service API (pkg/api/service.go)
participant UploadSvc as Upload Service (/v2/upload)
participant S3
participant DeploySvc as Prepare Endpoint (/v2/upload/{id}/prepare)
Client->>ServiceAPI: UploadZipToService(zipBytes, serviceID, envID)
ServiceAPI->>ServiceAPI: Compute SHA-256 of zipBytes\nBase64-encode => contentHash
ServiceAPI->>UploadSvc: POST /v2/upload {contentHash, algorithm, length}
UploadSvc-->>ServiceAPI: presign data (upload_id, url, method, headers)
ServiceAPI->>S3: PUT/POST to presigned URL with zip bytes
S3-->>ServiceAPI: upload response (status OK)
ServiceAPI->>DeploySvc: POST /v2/upload/{upload_id}/prepare {serviceID, environmentID}
DeploySvc-->>ServiceAPI: prepare confirmed
ServiceAPI-->>Client: return success / nil
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/api/service.go (1)
484-484: Return type mismatch: function always returns nil for service despite signature declaring*model.Service.The function signature at
pkg/api/service.go:372declares a return type of(*model.Service, error), but the implementation ends at line 484 withreturn nil, nil. The caller atinternal/cmd/deploy/deploy.go:99already discards the first return value with_, indicating this gap has been worked around rather than fixed.After completing all steps (hash calculation, upload session creation, S3 upload, and prepare deployment), the function exits without decoding the prepare response or returning the updated service object. Either the return type should be changed to
(error)only, or the function should fetch and return the service after the upload is prepared.
🤖 Fix all issues with AI agents
In `@pkg/api/deployment.go`:
- Around line 28-39: ListAllDeployments currently only fetches a single page of
5 results by calling ListDeployments(ctx, serviceID, environmentID, 0, 5) and
returning conn.Edges, which violates the function name; either implement proper
cursor-based pagination to aggregate all pages into model.Deployments (loop
calling ListDeployments using the returned page info/cursor until no more pages
and append edge.Node from each page) or rename the function to something like
ListRecentDeployments to reflect the limited fetch; update references to
ListAllDeployments accordingly and ensure you use the pagination metadata from
the DeploymentConnection response to drive the loop.
- Around line 9-10: The ListDeployments function currently accepts a skip
parameter but discards it by calling "_, limit = normalizePagination(skip,
limit)", which misleads callers; fix by honoring the returned offset: change the
call to capture the offset (e.g., "offset, limit := normalizePagination(skip,
limit)") and use that offset when constructing the request/query in
ListDeployments (update any downstream call that builds pagination parameters),
or if the API is cursor-based, remove the skip parameter from the
ListDeployments signature and adjust the interface and all callers accordingly;
reference the ListDeployments method and the normalizePagination helper when
making the change.
In `@pkg/fill/fill.go`:
- Around line 212-218: When resolving by serviceName inside
f.selector.SelectService call, don't overwrite the existing opt.FilterFunc;
instead wrap/compose it so the resulting FilterFunc returns true only if the
service name matches and the original opt.FilterFunc (if non-nil) also returns
true. Update the selector.SelectServiceOptions being passed (where FilterFunc is
currently set to func(s *model.Service) bool { return s.Name == *serviceName })
to call the original opt.FilterFunc(s) when present and combine it with the name
check so both constraints are enforced.
In `@pkg/model/deployment.go`:
- Around line 33-40: DeploymentConnection is missing the PageInfo field used by
other connection types; update the struct DeploymentConnection to include a
PageInfo *PageInfo field so it matches the generic Connection[T] pattern and
provides pagination metadata. Locate the DeploymentConnection definition and add
PageInfo *PageInfo (matching the PageInfo type used by
ProjectConnection/TemplateConnection in pageable.go), ensure JSON/graphQL tags
mirror the existing convention (e.g., `json:"pageInfo" graphql:"pageInfo"`), and
run tests/compilation to validate no other callers need adjustments.
🧹 Nitpick comments (2)
pkg/model/project.go (1)
96-99: Pre-existing type mismatch in constants.While not part of this PR's changes, these constants are typed as
ServiceTemplatebut should beRegionProviderbased on their naming and intended usage.♻️ Suggested fix
const ( - RegionProviderAWS ServiceTemplate = "AWS" - RegionProviderGCP ServiceTemplate = "GCP" + RegionProviderAWS RegionProvider = "AWS" + RegionProviderGCP RegionProvider = "GCP" )pkg/api/service.go (1)
396-403: Confusing variable names: request variables named as responses.
createUploadRespandprepareRespare*http.Requestobjects, not responses. This harms readability.Suggested rename
- createUploadResp, err := http.NewRequestWithContext(ctx, "POST", constant.ZeaburServerURL+"/v2/upload", bytes.NewReader(createUploadBody)) + createUploadReq, err := http.NewRequestWithContext(ctx, "POST", constant.ZeaburServerURL+"/v2/upload", bytes.NewReader(createUploadBody)) if err != nil { return nil, fmt.Errorf("failed to create upload request: %w", err) } token := viper.GetString("token") - createUploadResp.Header.Set("Content-Type", "application/json") - createUploadResp.Header.Set("Cookie", "token="+token) + createUploadReq.Header.Set("Content-Type", "application/json") + createUploadReq.Header.Set("Cookie", "token="+token) client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(createUploadResp) + resp, err := client.Do(createUploadReq)Apply similar fix for
prepareResp→prepareReqat lines 464-474.Also applies to: 464-472
pan93412
approved these changes
Jan 28, 2026
Member
|
你先修掉 CodeRabbit 指出的問題吧 |
Contributor
Author
有修改好了，第二個 issue 可以再請你看一下要如何處理 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (required)
deploymrnt.gotodeployment.goand updated the deployment API to use DeploymentConnection and DeploymentEdge for GraphQL connection pattern.Before:
After:
Related issues & labels (optional)
Summary by CodeRabbit
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.