| Age | Commit message (Collapse) | Author |
|---|
|
The variations of the Err<StatusCode>() and Err<StatusCode>f()
functions don't verify if the passed in error wraps gRPC error
already on some nested level except the upper one only.
That is why initially InvalidArgument code error could be
returned with Internal code and wrapped inside the message, so
it would look like there was another RPC method invocation.
The GrpcCode is refactored here as well, to follow the same approach.
Now it unwraps the errors and looks for the initial status most
deeply nested. If found it becomes the final for the RPC invocation
and send to the caller.
|
|
make: use 'override' when adding to TEST_OPTIONS
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4890
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
operations: Always use structured errors in UserCreateTag RPC
Closes #4372
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4882
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
doc: Clarify GitLab/Gitaly boundary for RPCs
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4878
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Andras Horvath <ahorvath@gitlab.com>
Approved-by: Mark Wood <mwood@gitlab.com>
|
|
Bump .tool-versions to use Go 1.18.6
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4892
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: Stan Hu <stanhu@gmail.com>
|
|
The GitLab Development Kit is moving to 1.18.6, so let's make all the
versions consistent to reduce the number of interpreters needed for
development.
Part of https://gitlab.com/groups/gitlab-org/-/epics/8843
|
|
go: Update module github.com/google/go-cmp to v0.5.9
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4863
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
When `TEST_OPTIONS` is specified as an argument to `make`, any options
specified in the `Makefile` will be overwritten with the user-provided
value. In several targets we append arguments to `TEST_OPTIONS`, making
them vulnerable to being squashed inadvertently.
For example, if running `make race-go TEST_OPTIONS='-run=<TEST>
-count=20'` to reproduce a race condition, the intended command would
be:
gotestsum --format short -- -run <TEST> -count 20 -race ./...
But because `TEST_OPTIONS` is specified, this is actually executed as:
gotestsum --format short -- -run <TEST> -count 20 ./...
Resolve this by using the `override` directive in targets that attempt
to append to `TEST_OPTIONS`. This will allow the user to add custom
options without squashing the target's additions.
|
|
'4455-feature-flag-roll-out-user_create_branch_structured_errors' into 'master'
operations: Structured errors in UserCreateBranch
Closes #4455
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4883
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
Revert "Merge branch...
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4888
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: karthik nayak <knayak@gitlab.com>
|
|
objectpool: Always prune refs on fetches to fix D/F conflicts
Closes #4394
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4885
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
|
|
With 018958fb1 (objectpool: Fix conflicting references when fetching
into pools, 2022-07-27) we have started to prune references in object
pools that have been removed in their primary pool member. This is a
necessity to avoid conflicting references in the object pool and its
member, which would otherwise break updating such pools completely.
This code has been rolled out to production on August 11th without any
observed negative fallout, and the flag was subsequently enabled by
default. Remove the feature flag guarding it.
Changelog: changed
|
|
golangci-lint: Stop excluding many `Close` and `Serve` functions from errcheck linter
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4810
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
|
|
'4452-feature-flag-roll-out-simplify_find_local_branches_response' into 'master'"
This reverts merge request !4884
|
|
We don't properly verify the error code when closing the file that
corresponds to the packed binary. While this doesn't really matter in
the first place, fixing this one case lets us drop another exception to
our linting rules.
|
|
Many of our tests don't properly verify the error returned by the server
factory's `Serve()` function. Fix this shortcoming, which surfaces that
we mistakenly close some listeners manually that should indeed be closed
by gRPC itself. Drop the corresponding linting exception.
|
|
The `http.Server.Serve()` function will return an error when the HTTP
server has either failed unexpectedly or in case it wasn't properly shut
down. We don't check this error in many places though, which both causes
the errcheck linter to complain and hides issues we have with some of
our test setups.
Fix the missing error checks via a new `testhelper.MustServe()` helper
function that does the error checking for us. Drop the corresponding
linting exceptions.
|
|
The `grpc.Server.Serve()` function will return an error when the gRPC
server has either failed unexpectedly or in case it wasn't properly shut
down. We don't check this error in many places though, which both causes
the errcheck linter to complain and hides issues we have with some of
our test setups.
Fix the missing error checks via a new `testhelper.MustServe()` helper
function that does the error checking for us. This surfaces some test
errors for tests that forget to shut down the server or that manually
close the listener, which should be handled by gRPC itself. All of those
errors are fixed while at it.
Drop the corresponding linting exceptions.
|
|
We don't properly check the error when closing the `SidechannelWaiter`
in many places. Fix this and drop the corresponding linting exceptions.
|
|
The `close()` function we return as part of the server handshake is
responsible for closing all associated resources. In case closing any of
them fails though we never bubble up errors at all, but instead silently
succeed.
Fix this and return the first error we encounter. As we now correctly
check returned errors the errcheck linter is happy and we can drop
another exclude for the `yamux.Session` type.
|
|
We already check the error when we manually close the zlib writer, but
don't in the deferred call. This is fine though at it won't do anything
in the happy path as the writer was closed already, and we already got
an error in the unhappy path.
Explicitly ignore the error to make our linters happy.
|
|
While we check error codes returned by the tar writer in the general
case when writing an archive, we ignore it in case we have already seen
an error in `TarBuilder.Close()`. We didn't explicitly ignore the error
with a blanket-assignment though, and thus the errcheck linter
complains.
Fix this by assigning the error to the blanket variable and drop the
corresponding exclude from our linting rules.
|
|
Check error codes when closing a `safe.LockingFileWriter` or
`safe.Writer` and drop the corresponding exclude from our linting rules.
|
|
Check the error code when closing the Unix socket listener and drop the
corresponding exclude from our linting rules.
|
|
The errcheck linter is tasked with finding cases where we are mistakenly
not checking errors returned by a function. While we have it globally
enabled, one gap that we currently have is that we exclude all function
calls to `Close` and `Serve`. This exception is something we added back
when we added this linter to our rules as it has proven to be too much
work to fix all violations in the code base. Not checking any such
errors may easily hide issues we're not aware of though and is thus bad
practice.
Convert the blanket-exclude for all functions to instead explicitly
exclude all functions that currently violate it. Like this, it becomes a
lot more manageable to fix missing error checks one function at a time
and makes sure that we don't add new violations to our codebase that are
not covered by any of these explicit function excludes.
|
|
'4452-feature-flag-roll-out-simplify_find_local_branches_response' into 'master'
ff: set SimplifyFindLocalBranchesResponse to true
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4884
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
With 2e642ccd (branches: Use 'UserCreateBranchError' behind a flag) we
have started using structured errors for the UserCreateBranch RPC
instead of relying on `Internal` errors.
This code was rolled out to production and the feature flag is currently
enabled by default. Let's remove the feature flag guarding it.
Changelog: changed
|
|
ref: Always use structured errors in FindTag RPC
Closes #4398
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4881
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Since this featureflag has a rails/gitlab counterpart we cannot simply
delete it. We need to ensure that both Gitaly and rails have it set to
true by default for one release and then we can delete the feature flag
in the next release.
|
|
With 0fedeb41f (operations: Introduce structured errors for
UserCreateTag, 2022-07-22) we have adapted the UserCreateTag RPC to
return structured errors instead of embedding errors in a successful
response. This both improves visibility into this RPC and fixes cases
where we needlessly create replication jobs due to missing votes.
This code has been rolled out to production on August 3rd without any
observed negative fallout. Remove the feature flag guarding it.
Changelog: changed
|
|
With 453ac57fc (ref: Fix `Internal` errors in `FindTag()` when tag
doesn't exist, 2022-08-01) we have adapted the FindTag RPC to return a
`NotFound` error instead of an `Internal` one when the tag wasn't found
in order to clearly label this error condition as expected.
This code has been rolled out to production on August 2nd without any
observed negative fallout. Remove the feature flag guarding it.
Changelog: changed
|
|
Makefile: Update Delve
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4879
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Toon Claes <toon@gitlab.com>
|
|
The previous version did not work with Go 1.19, so update it to the
latest version.
|
|
[ci skip]
|
|
In the past it wasn't always clear where exactly we put the boundary for
RPCs that should be implemented in Gitaly versus those that should be in
Rails. This was recently clarified though: Gitaly should always avoid to
implement business-specific logic and policy. Instead, its provided RPCs
should focus on the problem-domains specific to Gitaly and Praefect.
Clarify this in our documentation to make it more readily discoverable
by engineers.
|
|
Update pg_query to v2.1.4
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4872
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: Stan Hu <stanhu@gmail.com>
|
|
featureflag: Delete flag `GitV2371Gl1`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4874
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
|
|
|
|
go: Update module github.com/prometheus/client_golang to v1.13.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4814
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module go.uber.org/goleak to v1.2.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4868
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module github.com/golang-jwt/jwt/v4 to v4.4.2
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4840
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
The feature flag `GitV2371Gl1` is completely rolled out in production.
Now lets delete the flag. With this change we can follow up with a bump
to the minimum version of Git used in Gitlay.
|
|
This fixes an issue with compiling on macOS with XCode 14
(https://github.com/pganalyze/pg_query/pull/256) among other bugs.
- Diff: https://my.diffend.io/gems/pg_query/2.1.3/2.1.4
- Log: https://github.com/pganalyze/pg_query/blob/main/CHANGELOG.md
This mirrors the same change in GitLab Rails:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98274
Changelog: changed
|
|
featureflag: Ensure to only contain valid characters
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4793
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: blanet <moweng.xx@alibaba-inc.com>
|
|
metrics: Remove Praefect read-only metric
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4858
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
go: Update module github.com/jackc/pgx/v4 to v4.17.2
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4865
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
praefect: Read line-by-line in track-repositories
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4855
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
|
|
|
|
|
