From 51827294561cebf01a56b1bfe658d0a616303e2d Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Thu, 13 Feb 2025 09:52:04 -0400 Subject: [PATCH 1/2] [refactor] Use plain loops with iterators (#6722) ## Description of the changes - Follow-up to #6714 ## How was this change tested? - CI Signed-off-by: Yuri Shkuro --- cmd/query/app/apiv3/grpc_handler.go | 14 ++--- internal/jiter/iter.go | 24 ++------ internal/jiter/iter_test.go | 88 ++++++++++++++++++----------- 3 files changed, 66 insertions(+), 60 deletions(-) diff --git a/cmd/query/app/apiv3/grpc_handler.go b/cmd/query/app/apiv3/grpc_handler.go index 659c76b424a..271dd7b56bf 100644 --- a/cmd/query/app/apiv3/grpc_handler.go +++ b/cmd/query/app/apiv3/grpc_handler.go @@ -131,21 +131,17 @@ func receiveTraces( seq iter.Seq2[[]ptrace.Traces, error], sendFn func(*api_v3.TracesData) error, ) error { - var capturedErr error - seq(func(traces []ptrace.Traces, err error) bool { + for traces, err := range seq { if err != nil { - capturedErr = err - return false + return err } for _, trace := range traces { tracesData := api_v3.TracesData(trace) if err := sendFn(&tracesData); err != nil { - capturedErr = status.Error(codes.Internal, + return status.Error(codes.Internal, fmt.Sprintf("failed to send response stream chunk to client: %v", err)) - return false } } - return true - }) - return capturedErr + } + return nil } diff --git a/internal/jiter/iter.go b/internal/jiter/iter.go index f4e6e32ffb9..3a8db5818be 100644 --- a/internal/jiter/iter.go +++ b/internal/jiter/iter.go @@ -10,34 +10,22 @@ import ( func CollectWithErrors[V any](seq iter.Seq2[V, error]) ([]V, error) { var result []V - var err error - seq(func(v V, e error) bool { - if e != nil { - err = e - return false + for v, err := range seq { + if err != nil { + return nil, err } result = append(result, v) - return true - }) - if err != nil { - return nil, err } return result, nil } func FlattenWithErrors[V any](seq iter.Seq2[[]V, error]) ([]V, error) { var result []V - var err error - seq(func(v []V, e error) bool { - if e != nil { - err = e - return false + for v, err := range seq { + if err != nil { + return nil, err } result = append(result, v...) - return true - }) - if err != nil { - return nil, err } return result, nil } diff --git a/internal/jiter/iter_test.go b/internal/jiter/iter_test.go index c5a998d52d2..b25ec56d643 100644 --- a/internal/jiter/iter_test.go +++ b/internal/jiter/iter_test.go @@ -4,7 +4,6 @@ package jiter import ( - "iter" "testing" "github.com/stretchr/testify/assert" @@ -12,52 +11,64 @@ import ( ) func TestCollectWithErrors(t *testing.T) { + type item struct { + str string + err error + } tests := []struct { name string - seq iter.Seq2[string, error] + items []item expected []string err error }{ { name: "no errors", - seq: func(yield func(string, error) bool) { - yield("a", nil) - yield("b", nil) - yield("c", nil) + items: []item{ + {str: "a", err: nil}, + {str: "b", err: nil}, + {str: "c", err: nil}, }, expected: []string{"a", "b", "c"}, }, { name: "first error", - seq: func(yield func(string, error) bool) { - yield("a", nil) - yield("b", nil) - yield("c", assert.AnError) + items: []item{ + {str: "a", err: nil}, + {str: "b", err: nil}, + {str: "c", err: assert.AnError}, }, err: assert.AnError, }, { name: "second error", - seq: func(yield func(string, error) bool) { - yield("a", nil) - yield("b", assert.AnError) - yield("c", nil) + items: []item{ + {str: "a", err: nil}, + {str: "b", err: assert.AnError}, + {str: "c", err: nil}, }, err: assert.AnError, }, { name: "third error", - seq: func(yield func(string, error) bool) { - yield("a", nil) - yield("b", nil) - yield("c", assert.AnError) + items: []item{ + {str: "a", err: nil}, + {str: "b", err: nil}, + {str: "c", err: assert.AnError}, }, + err: assert.AnError, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result, err := CollectWithErrors(test.seq) + seq := func(yield func(string, error) bool) { + for _, item := range test.items { + if !yield(item.str, item.err) { + return + } + } + } + result, err := CollectWithErrors(seq) if test.err != nil { require.ErrorIs(t, err, test.err) } else { @@ -69,43 +80,54 @@ func TestCollectWithErrors(t *testing.T) { } func TestFlattenWithErrors(t *testing.T) { + type item struct { + strs []string + err error + } tests := []struct { name string - seq iter.Seq2[[]string, error] + items []item expected []string err error }{ { name: "no errors", - seq: func(yield func([]string, error) bool) { - yield([]string{"a", "b", "c"}, nil) - yield([]string{"d", "e", "f"}, nil) - yield([]string{"g", "h", "i"}, nil) + items: []item{ + {strs: []string{"a", "b", "c"}, err: nil}, + {strs: []string{"d", "e", "f"}, err: nil}, + {strs: []string{"g", "h", "i"}, err: nil}, }, expected: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i"}, }, { name: "first error", - seq: func(yield func([]string, error) bool) { - yield([]string{"a", "b", "c"}, nil) - yield([]string{"d", "e", "f"}, assert.AnError) - yield([]string{"g", "h", "i"}, nil) + items: []item{ + {strs: []string{"a", "b", "c"}, err: nil}, + {strs: []string{"d", "e", "f"}, err: assert.AnError}, + {strs: []string{"g", "h", "i"}, err: nil}, }, err: assert.AnError, }, { name: "second error", - seq: func(yield func([]string, error) bool) { - yield([]string{"a", "b", "c"}, nil) - yield([]string{"d", "e", "f"}, nil) - yield([]string{"g", "h", "i"}, assert.AnError) + items: []item{ + {strs: []string{"a", "b", "c"}, err: nil}, + {strs: []string{"d", "e", "f"}, err: nil}, + {strs: []string{"g", "h", "i"}, err: assert.AnError}, }, err: assert.AnError, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result, err := FlattenWithErrors(test.seq) + seq := func(yield func([]string, error) bool) { + for _, item := range test.items { + if !yield(item.strs, item.err) { + return + } + } + } + result, err := FlattenWithErrors(seq) if test.err != nil { require.ErrorIs(t, err, test.err) } else { From 4b884bbed339c26d23ef2c53fd350c8065f783f0 Mon Sep 17 00:00:00 2001 From: hippie-danish <133037056+danish9039@users.noreply.github.com> Date: Thu, 13 Feb 2025 20:08:47 +0530 Subject: [PATCH 2/2] Allow CI workflows to run from merge queue (#6719) ## Which problem is this PR solving? - Part of https://github.com/jaegertracing/jaeger/issues/6712 ## Description of the changes - replicate changes from jaeger-ui to jaeger ci-workflow for enabling merge queue feature ## How was this change tested? - make lint ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: danish9039 Signed-off-by: hippie-danish <133037056+danish9039@users.noreply.github.com> Co-authored-by: Yuri Shkuro --- .github/workflows/ci-build-binaries.yml | 1 + .github/workflows/ci-crossdock.yml | 1 + .github/workflows/ci-docker-all-in-one.yml | 1 + .github/workflows/ci-docker-build.yml | 1 + .github/workflows/ci-docker-hotrod.yml | 1 + .github/workflows/ci-e2e-all.yml | 1 + .github/workflows/ci-e2e-spm.yml | 1 + .../workflows/ci-e2e-tailsampling-processor.yml | 1 + .github/workflows/ci-lint-checks.yaml | 1 + .github/workflows/ci-lint-dependabot-config.yml | 1 + .github/workflows/ci-unit-tests-go-tip.yml | 1 + .github/workflows/ci-unit-tests.yml | 1 + .github/workflows/codeql.yml | 3 +++ .github/workflows/dco_merge_group.yml | 14 ++++++++++++++ .github/workflows/fossa.yml | 1 + .github/workflows/label-check.yml | 1 + 16 files changed, 31 insertions(+) create mode 100644 .github/workflows/dco_merge_group.yml diff --git a/.github/workflows/ci-build-binaries.yml b/.github/workflows/ci-build-binaries.yml index 9b357e39c0a..5b8556239a8 100644 --- a/.github/workflows/ci-build-binaries.yml +++ b/.github/workflows/ci-build-binaries.yml @@ -1,6 +1,7 @@ name: Build binaries on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-crossdock.yml b/.github/workflows/ci-crossdock.yml index c9fe99c9acb..95a1b30ff6e 100644 --- a/.github/workflows/ci-crossdock.yml +++ b/.github/workflows/ci-crossdock.yml @@ -1,6 +1,7 @@ name: CIT Crossdock on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-docker-all-in-one.yml b/.github/workflows/ci-docker-all-in-one.yml index b3d3e929049..e6ec1e0f287 100644 --- a/.github/workflows/ci-docker-all-in-one.yml +++ b/.github/workflows/ci-docker-all-in-one.yml @@ -1,6 +1,7 @@ name: Build all-in-one on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-docker-build.yml b/.github/workflows/ci-docker-build.yml index 4140e65b775..71d8bf39091 100644 --- a/.github/workflows/ci-docker-build.yml +++ b/.github/workflows/ci-docker-build.yml @@ -1,6 +1,7 @@ name: Build docker images on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-docker-hotrod.yml b/.github/workflows/ci-docker-hotrod.yml index 55d772cc996..5df220800b2 100644 --- a/.github/workflows/ci-docker-hotrod.yml +++ b/.github/workflows/ci-docker-hotrod.yml @@ -1,6 +1,7 @@ name: CIT Hotrod on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-e2e-all.yml b/.github/workflows/ci-e2e-all.yml index 082270f5488..b3cb3e0ad2e 100644 --- a/.github/workflows/ci-e2e-all.yml +++ b/.github/workflows/ci-e2e-all.yml @@ -1,6 +1,7 @@ name: E2E Tests on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-e2e-spm.yml b/.github/workflows/ci-e2e-spm.yml index 37ebd878e28..7c4185135cb 100644 --- a/.github/workflows/ci-e2e-spm.yml +++ b/.github/workflows/ci-e2e-spm.yml @@ -1,6 +1,7 @@ name: Test SPM on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-e2e-tailsampling-processor.yml b/.github/workflows/ci-e2e-tailsampling-processor.yml index f002e31faf4..4345c08652d 100644 --- a/.github/workflows/ci-e2e-tailsampling-processor.yml +++ b/.github/workflows/ci-e2e-tailsampling-processor.yml @@ -1,6 +1,7 @@ name: Test Tail Sampling Processor on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-lint-checks.yaml b/.github/workflows/ci-lint-checks.yaml index af812b2c6bd..5cfda304592 100644 --- a/.github/workflows/ci-lint-checks.yaml +++ b/.github/workflows/ci-lint-checks.yaml @@ -1,6 +1,7 @@ name: Lint Checks on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-lint-dependabot-config.yml b/.github/workflows/ci-lint-dependabot-config.yml index 0d8c69f4a1f..87ffecb374a 100644 --- a/.github/workflows/ci-lint-dependabot-config.yml +++ b/.github/workflows/ci-lint-dependabot-config.yml @@ -1,6 +1,7 @@ name: dependabot validate on: + merge_group: pull_request: paths: - '.github/dependabot.yml' diff --git a/.github/workflows/ci-unit-tests-go-tip.yml b/.github/workflows/ci-unit-tests-go-tip.yml index e153d331123..b367c89f9f9 100644 --- a/.github/workflows/ci-unit-tests-go-tip.yml +++ b/.github/workflows/ci-unit-tests-go-tip.yml @@ -1,6 +1,7 @@ name: Unit Tests on Go Tip on: + merge_group: push: branches: [main] diff --git a/.github/workflows/ci-unit-tests.yml b/.github/workflows/ci-unit-tests.yml index 67a279570fa..2a025b4308b 100644 --- a/.github/workflows/ci-unit-tests.yml +++ b/.github/workflows/ci-unit-tests.yml @@ -1,6 +1,7 @@ name: Unit Tests on: + merge_group: push: branches: [main] diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index e6179b880d0..1d5194b920d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,6 +1,7 @@ name: "CodeQL" on: + merge_group: push: branches: [main] @@ -20,6 +21,8 @@ permissions: # added using https://github.com/step-security/secure-workflows jobs: codeql-analyze: + # Run only on pull requests, see https://github.com/github/codeql-action/issues/1537 + if: ${{ github.event_name == 'pull_request' }} name: CodeQL Analyze runs-on: ubuntu-latest diff --git a/.github/workflows/dco_merge_group.yml b/.github/workflows/dco_merge_group.yml new file mode 100644 index 00000000000..e570c765320 --- /dev/null +++ b/.github/workflows/dco_merge_group.yml @@ -0,0 +1,14 @@ +# Fake "DCO check" workflow inspired by https://github.com/onnx/onnx/pull/5398/files. +# The regular DCO check is required, but it does not run from a merge queue and there is +# no way to configure it to run. +name: DCO +on: + merge_group: + +permissions: + contents: read +jobs: + DCO: + runs-on: ubuntu-latest + steps: + - run: echo "Fake DCO check to avoid blocking the merge queue" \ No newline at end of file diff --git a/.github/workflows/fossa.yml b/.github/workflows/fossa.yml index 996aea337b8..9d9284114c5 100644 --- a/.github/workflows/fossa.yml +++ b/.github/workflows/fossa.yml @@ -1,6 +1,7 @@ name: FOSSA on: + merge_group: push: branches: [main] diff --git a/.github/workflows/label-check.yml b/.github/workflows/label-check.yml index acbaf2a5323..eb49f5c6c05 100644 --- a/.github/workflows/label-check.yml +++ b/.github/workflows/label-check.yml @@ -1,6 +1,7 @@ name: Verify PR Label on: + merge_group: pull_request: types: - opened