From 1ae0d09e10d299ae40fafd2578f10d3024c9275b Mon Sep 17 00:00:00 2001 From: Archish Date: Mon, 2 Sep 2024 02:13:41 +0530 Subject: [PATCH 1/5] Lint issue in gitaly --- .../_support/lint_last_known_acceptable.txt | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt index 48d5c84f8c732a..89acaa65a500a8 100644 --- a/workhorse/_support/lint_last_known_acceptable.txt +++ b/workhorse/_support/lint_last_known_acceptable.txt @@ -109,32 +109,6 @@ internal/git/responsewriter.go:52:1: exported: exported method HTTPResponseWrite internal/git/snapshot.go:27:2: exported: exported var SendSnapshot should have comment or be unexported (revive) internal/git/upload-pack.go:37:16: Error return value of `cw.Flush` is not checked (errcheck) internal/git/upload-pack_test.go:72:2: error-is-as: use require.ErrorIs (testifylint) -internal/gitaly/blob.go:1:1: package-comments: should have a package comment (revive) -internal/gitaly/blob.go:14:6: exported: exported type BlobClient should have comment or be unexported (revive) -internal/gitaly/blob.go:18:1: exported: exported method BlobClient.SendBlob should have comment or be unexported (revive) -internal/gitaly/diff.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/diff.go:13:6: exported: exported type DiffClient should have comment or be unexported (revive) -internal/gitaly/diff.go:17: 17-35 lines are duplicate of `internal/gitaly/diff.go:37-55` (dupl) -internal/gitaly/diff.go:37: 37-55 lines are duplicate of `internal/gitaly/diff.go:17-35` (dupl) -internal/gitaly/gitaly.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/gitaly.go:63:1: exported: exported function InitializeSidechannelRegistry should have comment or be unexported (revive) -internal/gitaly/gitaly.go:85:1: exported: exported function NewSmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:98:1: exported: exported function NewBlobClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:107:1: exported: exported function NewRepositoryClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:116:1: exported: exported function NewDiffClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:155:5: shadow: declaration of "conn" shadows declaration at line 145 (govet) -internal/gitaly/gitaly.go:169:1: exported: exported function CloseConnections should have comment or be unexported (revive) -internal/gitaly/gitaly.go:174:13: Error return value of `conn.Close` is not checked (errcheck) -internal/gitaly/gitaly.go:179:14: appendAssign: append result not assigned to the same slice (gocritic) -internal/gitaly/gitaly.go:219:1: exported: exported function UnmarshalJSON should have comment or be unexported (revive) -internal/gitaly/repository.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:13:6: exported: exported type SmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:18:1: exported: exported method SmartHTTPClient.InfoRefsResponseReader should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:48:1: exported: exported method SmartHTTPClient.ReceivePack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:84:19: Error return value of `stream.CloseSend` is not checked (errcheck) -internal/gitaly/smarthttp.go:97:1: exported: exported method SmartHTTPClient.UploadPack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:113:20: Error return value of `waiter.Close` is not checked (errcheck) internal/headers/headers.go:10: internal/headers/headers.go:10: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "Fixme: Go back to 512 bytes once https:/..." (godox) internal/helper/exception/exception.go:11:14: SA1019: "gitlab.com/gitlab-org/labkit/correlation/raven" is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) internal/helper/exception/exception.go:36:11: SA1019: correlation.SetExtra is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) -- GitLab From 7895b61a8674ffd603b569d07e8efe0ce4291d72 Mon Sep 17 00:00:00 2001 From: Archish Date: Tue, 24 Sep 2024 22:51:47 +0530 Subject: [PATCH 2/5] Lint fixes --- .../_support/lint_last_known_acceptable.txt | 27 ++------------ workhorse/internal/gitaly/blob.go | 3 ++ workhorse/internal/gitaly/diff.go | 36 +++++++++---------- workhorse/internal/gitaly/gitaly.go | 20 +++++++---- workhorse/internal/gitaly/repository.go | 1 + workhorse/internal/gitaly/smarthttp.go | 10 ++++-- 6 files changed, 46 insertions(+), 51 deletions(-) diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt index 0f76c6a833fcda..6a1572a02ac433 100644 --- a/workhorse/_support/lint_last_known_acceptable.txt +++ b/workhorse/_support/lint_last_known_acceptable.txt @@ -86,31 +86,8 @@ internal/git/responsewriter.go:52:1: exported: exported method HTTPResponseWrite internal/git/snapshot.go:27:2: exported: exported var SendSnapshot should have comment or be unexported (revive) internal/git/upload-pack.go:37:16: Error return value of `cw.Flush` is not checked (errcheck) internal/git/upload-pack_test.go:72:2: error-is-as: use require.ErrorIs (testifylint) -internal/gitaly/blob.go:1:1: package-comments: should have a package comment (revive) -internal/gitaly/blob.go:14:6: exported: exported type BlobClient should have comment or be unexported (revive) -internal/gitaly/blob.go:18:1: exported: exported method BlobClient.SendBlob should have comment or be unexported (revive) -internal/gitaly/diff.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/diff.go:13:6: exported: exported type DiffClient should have comment or be unexported (revive) -internal/gitaly/diff.go:17: 17-35 lines are duplicate of `internal/gitaly/diff.go:37-55` (dupl) -internal/gitaly/diff.go:37: 37-55 lines are duplicate of `internal/gitaly/diff.go:17-35` (dupl) -internal/gitaly/gitaly.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/gitaly.go:63:1: exported: exported function InitializeSidechannelRegistry should have comment or be unexported (revive) -internal/gitaly/gitaly.go:85:1: exported: exported function NewSmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:98:1: exported: exported function NewBlobClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:107:1: exported: exported function NewRepositoryClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:116:1: exported: exported function NewDiffClient should have comment or be unexported (revive) -internal/gitaly/gitaly.go:169:1: exported: exported function CloseConnections should have comment or be unexported (revive) -internal/gitaly/gitaly.go:174:13: Error return value of `conn.Close` is not checked (errcheck) -internal/gitaly/gitaly.go:179:14: appendAssign: append result not assigned to the same slice (gocritic) -internal/gitaly/gitaly.go:219:1: exported: exported function UnmarshalJSON should have comment or be unexported (revive) -internal/gitaly/repository.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:1:1: ST1000: at least one file in a package should have a package comment (stylecheck) -internal/gitaly/smarthttp.go:13:6: exported: exported type SmartHTTPClient should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:18:1: exported: exported method SmartHTTPClient.InfoRefsResponseReader should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:48:1: exported: exported method SmartHTTPClient.ReceivePack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:84:19: Error return value of `stream.CloseSend` is not checked (errcheck) -internal/gitaly/smarthttp.go:97:1: exported: exported method SmartHTTPClient.UploadPack should have comment or be unexported (revive) -internal/gitaly/smarthttp.go:113:20: Error return value of `waiter.Close` is not checked (errcheck) +internal/gitaly/gitaly.go:181:3: G104: Errors unhandled. (gosec) +internal/gitaly/smarthttp.go:89:3: G104: Errors unhandled. (gosec) internal/headers/headers.go:10: internal/headers/headers.go:10: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "Fixme: Go back to 512 bytes once https:/..." (godox) internal/helper/exception/exception.go:11:14: SA1019: "gitlab.com/gitlab-org/labkit/correlation/raven" is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) internal/helper/exception/exception.go:36:11: SA1019: correlation.SetExtra is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) diff --git a/workhorse/internal/gitaly/blob.go b/workhorse/internal/gitaly/blob.go index c84c1731300ad0..fd24a0a2216859 100644 --- a/workhorse/internal/gitaly/blob.go +++ b/workhorse/internal/gitaly/blob.go @@ -1,3 +1,4 @@ +// Package gitaly provides a client to interact with the blob service. package gitaly import ( @@ -11,10 +12,12 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// BlobClient wraps the gRPC client for Gitaly's BlobService. type BlobClient struct { gitalypb.BlobServiceClient } +// SendBlob streams the blob data from Gitaly to the HTTP response writer. func (client *BlobClient) SendBlob(ctx context.Context, w http.ResponseWriter, request *gitalypb.GetBlobRequest) error { c, err := client.GetBlob(ctx, request) if err != nil { diff --git a/workhorse/internal/gitaly/diff.go b/workhorse/internal/gitaly/diff.go index a8f427231bbb55..e50817dba54af1 100644 --- a/workhorse/internal/gitaly/diff.go +++ b/workhorse/internal/gitaly/diff.go @@ -1,3 +1,4 @@ +// Package gitaly provides a client to stream diffs and patches. package gitaly import ( @@ -10,22 +11,15 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// DiffClient wraps the Gitaly DiffServiceClient. type DiffClient struct { gitalypb.DiffServiceClient } -func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawDiffRequest) error { - c, err := client.RawDiff(ctx, request) - if err != nil { - return fmt.Errorf("rpc failed: %v", err) - } - +func (client *DiffClient) sendStream(w http.ResponseWriter, recv func() ([]byte, error)) error { w.Header().Del("Content-Length") - rr := streamio.NewReader(func() ([]byte, error) { - resp, err := c.Recv() - return resp.GetData(), err - }) + rr := streamio.NewReader(recv) if _, err := io.Copy(w, rr); err != nil { return fmt.Errorf("copy rpc data: %v", err) @@ -34,22 +28,28 @@ func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter return nil } -func (client *DiffClient) SendRawPatch(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawPatchRequest) error { - c, err := client.RawPatch(ctx, request) +// SendRawDiff streams a raw diff to the HTTP response. +func (client *DiffClient) SendRawDiff(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawDiffRequest) error { + c, err := client.RawDiff(ctx, request) if err != nil { return fmt.Errorf("rpc failed: %v", err) } - w.Header().Del("Content-Length") - - rr := streamio.NewReader(func() ([]byte, error) { + return client.sendStream(w, func() ([]byte, error) { resp, err := c.Recv() return resp.GetData(), err }) +} - if _, err := io.Copy(w, rr); err != nil { - return fmt.Errorf("copy rpc data: %v", err) +// SendRawPatch streams a raw patch to the HTTP response. +func (client *DiffClient) SendRawPatch(ctx context.Context, w http.ResponseWriter, request *gitalypb.RawPatchRequest) error { + c, err := client.RawPatch(ctx, request) + if err != nil { + return fmt.Errorf("rpc failed: %v", err) } - return nil + return client.sendStream(w, func() ([]byte, error) { + resp, err := c.Recv() + return resp.GetData(), err + }) } diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go index e4e2c827e9407d..94c5f6a890565d 100644 --- a/workhorse/internal/gitaly/gitaly.go +++ b/workhorse/internal/gitaly/gitaly.go @@ -1,3 +1,4 @@ +// Package gitaly provides a client to manage Gitaly connections. package gitaly import ( @@ -60,6 +61,7 @@ var ( ) ) +// InitializeSidechannelRegistry initializes if not already initialized. func InitializeSidechannelRegistry(logger *logrus.Logger) { if sidechannelRegistry == nil { sidechannelRegistry = gitalyclient.NewSidechannelRegistry(logrus.NewEntry(logger)) @@ -82,6 +84,7 @@ func withOutgoingMetadata(ctx context.Context, gs api.GitalyServer) context.Cont return metadata.NewOutgoingContext(ctx, md) } +// NewSmartHTTPClient is created and returns it with updated context. func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.Context, *SmartHTTPClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -95,6 +98,7 @@ func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.C return withOutgoingMetadata(ctx, server), smartHTTPClient, nil } +// NewBlobClient is created and returns it with updated context. func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Context, *BlobClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -104,6 +108,7 @@ func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Contex return withOutgoingMetadata(ctx, server), &BlobClient{grpcClient}, nil } +// NewRepositoryClient is created and returns it with updated context. func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context.Context, *RepositoryClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -113,6 +118,7 @@ func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context. return withOutgoingMetadata(ctx, server), &RepositoryClient{grpcClient}, nil } +// NewDiffClient is created and returns it with updated context. func NewDiffClient(ctx context.Context, server api.GitalyServer) (context.Context, *DiffClient, error) { conn, err := getOrCreateConnection(server) if err != nil { @@ -156,27 +162,29 @@ func getOrCreateConnection(server api.GitalyServer) (*grpc.ClientConn, error) { return cachedConn, nil } - conn, err := newConnection(server) + newConn, err := newConnection(server) if err != nil { return nil, err } - cache.connections[key] = conn + cache.connections[key] = newConn - return conn, nil + return newConn, nil } +// CloseConnections closes all connections in cache. func CloseConnections() { cache.Lock() defer cache.Unlock() for _, conn := range cache.connections { - conn.Close() + conn.Close() //nolint:errcheck } } func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { - connOpts := append(gitalyclient.DefaultDialOpts, + connOpts := gitalyclient.DefaultDialOpts + connOpts = append(connOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(server.Token)), grpc.WithChainStreamInterceptor( grpctracing.StreamClientTracingInterceptor(), @@ -193,7 +201,6 @@ func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { grpccorrelation.WithClientName("gitlab-workhorse"), ), ), - // In https://gitlab.com/groups/gitlab-org/-/epics/8971, we added DNS discovery support to Praefect. This was // done by making two changes: // - Configure client-side round-robin load-balancing in client dial options. We added that as a default option @@ -216,6 +223,7 @@ func newConnection(server api.GitalyServer) (*grpc.ClientConn, error) { return conn, connErr } +// UnmarshalJSON into a protobuf message. func UnmarshalJSON(s string, msg proto.Message) error { return protojson.UnmarshalOptions{DiscardUnknown: true}.Unmarshal([]byte(s), msg) } diff --git a/workhorse/internal/gitaly/repository.go b/workhorse/internal/gitaly/repository.go index ad7ef869316083..ac1b5f1f57b80a 100644 --- a/workhorse/internal/gitaly/repository.go +++ b/workhorse/internal/gitaly/repository.go @@ -1,3 +1,4 @@ +// Package gitaly provides a client for Gitaly service operations on Git repositories. package gitaly import ( diff --git a/workhorse/internal/gitaly/smarthttp.go b/workhorse/internal/gitaly/smarthttp.go index 2fdf88831b22ff..e67d8f7753b3a1 100644 --- a/workhorse/internal/gitaly/smarthttp.go +++ b/workhorse/internal/gitaly/smarthttp.go @@ -1,3 +1,5 @@ +// Package gitaly provides a client for interacting with Gitaly services, +// including smart HTTP requests for Git repository operations. package gitaly import ( @@ -10,11 +12,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/streamio" ) +// SmartHTTPClient encapsulates the SmartHTTPServiceClient for Gitaly. type SmartHTTPClient struct { sidechannelRegistry *gitalyclient.SidechannelRegistry gitalypb.SmartHTTPServiceClient } +// InfoRefsResponseReader handles InfoRefs requests and returns an io.Reader for the response. func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *gitalypb.Repository, rpc string, gitConfigOptions []string, gitProtocol string) (io.Reader, error) { rpcRequest := &gitalypb.InfoRefsRequest{ Repository: repo, @@ -45,6 +49,7 @@ func infoRefsReader(stream infoRefsClient) io.Reader { }) } +// ReceivePack performs a receive pack operation with Git configuration options. func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.Repository, glID string, glUsername string, glRepository string, gitConfigOptions []string, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error { stream, err := client.PostReceivePack(ctx) if err != nil { @@ -81,7 +86,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R return stream.Send(&gitalypb.PostReceivePackRequest{Data: data}) }) _, err := io.Copy(sw, clientRequest) - stream.CloseSend() + stream.CloseSend() //nolint:errcheck errC <- err }() @@ -94,6 +99,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R return nil } +// UploadPack performs an upload pack operation with a sidechannel. func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string, gitProtocol string) (*gitalypb.PostUploadPackWithSidechannelResponse, error) { ctx, waiter := client.sidechannelRegistry.Register(ctx, func(conn gitalyclient.SidechannelConn) error { if _, err := io.Copy(conn, clientRequest); err != nil { @@ -110,7 +116,7 @@ func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Re return nil }) - defer waiter.Close() + defer waiter.Close() //nolint:errcheck rpcRequest := &gitalypb.PostUploadPackWithSidechannelRequest{ Repository: repo, -- GitLab From 99f0bd513c3945097e98011c0622ad7bf7ebd5b9 Mon Sep 17 00:00:00 2001 From: Archish Date: Sat, 28 Sep 2024 23:07:44 +0530 Subject: [PATCH 3/5] Handled lint fixes & package comments --- workhorse/_support/lint_last_known_acceptable.txt | 2 -- workhorse/internal/gitaly/blob.go | 9 ++++++++- workhorse/internal/gitaly/diff.go | 11 ++++++++++- workhorse/internal/gitaly/gitaly.go | 8 +++++--- workhorse/internal/gitaly/repository.go | 5 ++++- workhorse/internal/gitaly/smarthttp.go | 6 ++++-- 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt index 6a1572a02ac433..9a07a95e6628c6 100644 --- a/workhorse/_support/lint_last_known_acceptable.txt +++ b/workhorse/_support/lint_last_known_acceptable.txt @@ -86,8 +86,6 @@ internal/git/responsewriter.go:52:1: exported: exported method HTTPResponseWrite internal/git/snapshot.go:27:2: exported: exported var SendSnapshot should have comment or be unexported (revive) internal/git/upload-pack.go:37:16: Error return value of `cw.Flush` is not checked (errcheck) internal/git/upload-pack_test.go:72:2: error-is-as: use require.ErrorIs (testifylint) -internal/gitaly/gitaly.go:181:3: G104: Errors unhandled. (gosec) -internal/gitaly/smarthttp.go:89:3: G104: Errors unhandled. (gosec) internal/headers/headers.go:10: internal/headers/headers.go:10: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "Fixme: Go back to 512 bytes once https:/..." (godox) internal/helper/exception/exception.go:11:14: SA1019: "gitlab.com/gitlab-org/labkit/correlation/raven" is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) internal/helper/exception/exception.go:36:11: SA1019: correlation.SetExtra is deprecated: Use gitlab.com/gitlab-org/labkit/errortracking instead. (staticcheck) diff --git a/workhorse/internal/gitaly/blob.go b/workhorse/internal/gitaly/blob.go index fd24a0a2216859..88d230ec0b9753 100644 --- a/workhorse/internal/gitaly/blob.go +++ b/workhorse/internal/gitaly/blob.go @@ -1,4 +1,11 @@ -// Package gitaly provides a client to interact with the blob service. +/* +Package gitaly provides a client for interacting with the Gitaly Blob service. + +This package allows for streaming blob data from Gitaly using the BlobServiceClient. +It facilitates the retrieval of blob data via gRPC and supports sending the data +directly to an HTTP response writer, making it suitable for serving blob content +over HTTP requests. +*/ package gitaly import ( diff --git a/workhorse/internal/gitaly/diff.go b/workhorse/internal/gitaly/diff.go index e50817dba54af1..63a128fc7285cf 100644 --- a/workhorse/internal/gitaly/diff.go +++ b/workhorse/internal/gitaly/diff.go @@ -1,4 +1,13 @@ -// Package gitaly provides a client to stream diffs and patches. +/* +Package gitaly provides a client to stream diffs and patches from the Gitaly server. + +This package wraps the Gitaly DiffServiceClient, allowing for the retrieval and +streaming of raw diffs and patches over HTTP. It provides methods to handle +streaming responses efficiently, ensuring that data is copied directly to +the HTTP response writer. The client is designed to facilitate operations +related to version control diffs and patches, enhancing interaction with +Git repositories managed by Gitaly. +*/ package gitaly import ( diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go index 94c5f6a890565d..dfa3ab922dffa5 100644 --- a/workhorse/internal/gitaly/gitaly.go +++ b/workhorse/internal/gitaly/gitaly.go @@ -1,4 +1,6 @@ -// Package gitaly provides a client to manage Gitaly connections. +// Package gitaly manages gRPC connections to the Gitaly server, providing clients +// for various services like Blob, Repository, and Diff, along with connection +// caching and metadata handling. package gitaly import ( @@ -61,7 +63,7 @@ var ( ) ) -// InitializeSidechannelRegistry initializes if not already initialized. +// InitializeSidechannelRegistry creates the side channel registry if it doesn't exist. func InitializeSidechannelRegistry(logger *logrus.Logger) { if sidechannelRegistry == nil { sidechannelRegistry = gitalyclient.NewSidechannelRegistry(logrus.NewEntry(logger)) @@ -178,7 +180,7 @@ func CloseConnections() { defer cache.Unlock() for _, conn := range cache.connections { - conn.Close() //nolint:errcheck + _ = conn.Close() } } diff --git a/workhorse/internal/gitaly/repository.go b/workhorse/internal/gitaly/repository.go index ac1b5f1f57b80a..4d9a6de939403e 100644 --- a/workhorse/internal/gitaly/repository.go +++ b/workhorse/internal/gitaly/repository.go @@ -1,4 +1,7 @@ -// Package gitaly provides a client for Gitaly service operations on Git repositories. +// Package gitaly provides a client for Gitaly service operations on Git repositories, +// including functionality to retrieve repository archives and snapshots. +// It encapsulates gRPC calls to the RepositoryService, allowing for streaming +// data responses through io.Reader interfaces. package gitaly import ( diff --git a/workhorse/internal/gitaly/smarthttp.go b/workhorse/internal/gitaly/smarthttp.go index e67d8f7753b3a1..757c0ee6d43e5d 100644 --- a/workhorse/internal/gitaly/smarthttp.go +++ b/workhorse/internal/gitaly/smarthttp.go @@ -1,5 +1,7 @@ // Package gitaly provides a client for interacting with Gitaly services, -// including smart HTTP requests for Git repository operations. +// enabling operations on Git repositories through smart HTTP requests. +// It includes functionality for handling InfoRefs, ReceivePack, and UploadPack +// operations while supporting streaming of request and response data. package gitaly import ( @@ -86,7 +88,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R return stream.Send(&gitalypb.PostReceivePackRequest{Data: data}) }) _, err := io.Copy(sw, clientRequest) - stream.CloseSend() //nolint:errcheck + _ = stream.CloseSend() errC <- err }() -- GitLab From 7bbb30fbb8247e931a38fabad0e77bc8cd6da3eb Mon Sep 17 00:00:00 2001 From: Archish Date: Tue, 1 Oct 2024 10:31:09 +0530 Subject: [PATCH 4/5] Updated comment --- workhorse/internal/gitaly/blob.go | 2 +- workhorse/internal/gitaly/diff.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/workhorse/internal/gitaly/blob.go b/workhorse/internal/gitaly/blob.go index 88d230ec0b9753..b49961bcb1ed87 100644 --- a/workhorse/internal/gitaly/blob.go +++ b/workhorse/internal/gitaly/blob.go @@ -1,7 +1,7 @@ /* Package gitaly provides a client for interacting with the Gitaly Blob service. -This package allows for streaming blob data from Gitaly using the BlobServiceClient. +This file allows for streaming blob data from Gitaly using the BlobServiceClient. It facilitates the retrieval of blob data via gRPC and supports sending the data directly to an HTTP response writer, making it suitable for serving blob content over HTTP requests. diff --git a/workhorse/internal/gitaly/diff.go b/workhorse/internal/gitaly/diff.go index 63a128fc7285cf..31a40d05f622ec 100644 --- a/workhorse/internal/gitaly/diff.go +++ b/workhorse/internal/gitaly/diff.go @@ -1,7 +1,7 @@ /* Package gitaly provides a client to stream diffs and patches from the Gitaly server. -This package wraps the Gitaly DiffServiceClient, allowing for the retrieval and +This file wraps the Gitaly DiffServiceClient, allowing for the retrieval and streaming of raw diffs and patches over HTTP. It provides methods to handle streaming responses efficiently, ensuring that data is copied directly to the HTTP response writer. The client is designed to facilitate operations -- GitLab From 3930f3097c6fbcd427e401c892d067f46a33218d Mon Sep 17 00:00:00 2001 From: Archish Date: Wed, 2 Oct 2024 15:21:59 +0530 Subject: [PATCH 5/5] Removed package comments --- workhorse/internal/gitaly/blob.go | 8 -------- workhorse/internal/gitaly/diff.go | 10 ---------- workhorse/internal/gitaly/gitaly.go | 16 +++++++++++++--- workhorse/internal/gitaly/repository.go | 4 ---- workhorse/internal/gitaly/smarthttp.go | 4 ---- 5 files changed, 13 insertions(+), 29 deletions(-) diff --git a/workhorse/internal/gitaly/blob.go b/workhorse/internal/gitaly/blob.go index b49961bcb1ed87..f5a7f82ce6aff5 100644 --- a/workhorse/internal/gitaly/blob.go +++ b/workhorse/internal/gitaly/blob.go @@ -1,11 +1,3 @@ -/* -Package gitaly provides a client for interacting with the Gitaly Blob service. - -This file allows for streaming blob data from Gitaly using the BlobServiceClient. -It facilitates the retrieval of blob data via gRPC and supports sending the data -directly to an HTTP response writer, making it suitable for serving blob content -over HTTP requests. -*/ package gitaly import ( diff --git a/workhorse/internal/gitaly/diff.go b/workhorse/internal/gitaly/diff.go index 31a40d05f622ec..dc036970f0f37d 100644 --- a/workhorse/internal/gitaly/diff.go +++ b/workhorse/internal/gitaly/diff.go @@ -1,13 +1,3 @@ -/* -Package gitaly provides a client to stream diffs and patches from the Gitaly server. - -This file wraps the Gitaly DiffServiceClient, allowing for the retrieval and -streaming of raw diffs and patches over HTTP. It provides methods to handle -streaming responses efficiently, ensuring that data is copied directly to -the HTTP response writer. The client is designed to facilitate operations -related to version control diffs and patches, enhancing interaction with -Git repositories managed by Gitaly. -*/ package gitaly import ( diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go index dfa3ab922dffa5..887f71815ac04f 100644 --- a/workhorse/internal/gitaly/gitaly.go +++ b/workhorse/internal/gitaly/gitaly.go @@ -1,6 +1,16 @@ -// Package gitaly manages gRPC connections to the Gitaly server, providing clients -// for various services like Blob, Repository, and Diff, along with connection -// caching and metadata handling. +/* +Package gitaly provides a comprehensive client for interacting with Gitaly services, facilitating operations on Git repositories. Key features include: + +1. Streaming blob data from Gitaly Blob service, suitable for serving content over HTTP. +2. Retrieving and streaming diffs and patches from Gitaly server. +3. Managing gRPC connections to Gitaly server with connection caching and metadata handling. +4. Providing clients for various services including Blob, Repository, and Diff. +5. Retrieving repository archives and snapshots through the RepositoryService. +6. Handling Git operations via smart HTTP requests, including InfoRefs, ReceivePack, and UploadPack. +7. Supporting efficient streaming of request and response data. + +This package enhances interaction with Git repositories managed by Gitaly, offering a streamlined interface for version control operations and data retrieval. +*/ package gitaly import ( diff --git a/workhorse/internal/gitaly/repository.go b/workhorse/internal/gitaly/repository.go index 4d9a6de939403e..ad7ef869316083 100644 --- a/workhorse/internal/gitaly/repository.go +++ b/workhorse/internal/gitaly/repository.go @@ -1,7 +1,3 @@ -// Package gitaly provides a client for Gitaly service operations on Git repositories, -// including functionality to retrieve repository archives and snapshots. -// It encapsulates gRPC calls to the RepositoryService, allowing for streaming -// data responses through io.Reader interfaces. package gitaly import ( diff --git a/workhorse/internal/gitaly/smarthttp.go b/workhorse/internal/gitaly/smarthttp.go index 757c0ee6d43e5d..e9484676707fb2 100644 --- a/workhorse/internal/gitaly/smarthttp.go +++ b/workhorse/internal/gitaly/smarthttp.go @@ -1,7 +1,3 @@ -// Package gitaly provides a client for interacting with Gitaly services, -// enabling operations on Git repositories through smart HTTP requests. -// It includes functionality for handling InfoRefs, ReceivePack, and UploadPack -// operations while supporting streaming of request and response data. package gitaly import ( -- GitLab