Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

batches: implement pagination for batch_spec_workspace_steps.output_lines#46335

Merged
BolajiOlajide merged 30 commits intomainfrom
bo/fix-pagination-output-lines
Jan 31, 2023
Merged

batches: implement pagination for batch_spec_workspace_steps.output_lines#46335
BolajiOlajide merged 30 commits intomainfrom
bo/fix-pagination-output-lines

Conversation

@BolajiOlajide
Copy link
Contributor

@BolajiOlajide BolajiOlajide commented Jan 11, 2023

Reported here.
Closes #1800

A customer reported that steps in their spec output were truncated. Started debugging by trying this out with src-cli and inspecting the log output to confirm if the output was truncated from src-cli; it wasn't.
I then proceeded to debug via SSBC route and discovered we stored the complete output in batch_spec_workspace_execution_jobs.execution_logs and found out the resolver for BatchSpecWorkspace steps was paginated and this meant that when using the ${{ previous_step.stdout }} directive one wouldn't get the complete output from the previous steps and the output displayed was truncated.

The initial value for slicing the output lines was 500, I'm not 100% certain why that limit was placed but this PR removes the limit so we can get all lines in every step and also display the complete logs for users.

CleanShot 2023-01-29 at 01 02 56

Test plan

Tested with a sample batch spec

name: brevity-2
description: Add Hello World to READMEs

# Find the first 100 search results for README files.
# This could yield less than 100 repos/workspaces if some repos contain multiple READMEs.
on:
  - repository: github.com/sourcegraph-testing/markdowns

# In each repository, run this command. Each repository's resulting diff is captured.
steps:
  - run: |
        for i in $(seq 1 1000); do
          echo "Hello world: $i"
        done
    container: alpine:3

  - run: |
      cat /tmp/stuff
    container: alpine:3
    files:
      /tmp/stuff: |
        ${{ previous_step.stdout }}

# Describe the changeset (e.g., GitHub pull request) you want for each repository.
changesetTemplate:
  title: Brevity test
  body: My first batch change!
  branch: brev
  commit:
    message: Testing Testing

Confirmed all output lines are displayed.

Loom

@BolajiOlajide BolajiOlajide added the batch-changes Issues related to Batch Changes label Jan 11, 2023
@BolajiOlajide BolajiOlajide requested a review from a team January 11, 2023 16:08
@BolajiOlajide BolajiOlajide self-assigned this Jan 11, 2023
@cla-bot cla-bot bot added the cla-signed label Jan 11, 2023
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 11, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff b4a20e9...e854bf3.

Notify File(s)
@courier-new client/web/src/enterprise/batches/batch-spec/batch-spec.mock.ts
client/web/src/enterprise/batches/batch-spec/execute/backend.ts
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.module.scss
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.story.tsx
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.tsx
cmd/frontend/graphqlbackend/batches.go
@eseliger client/web/src/enterprise/batches/batch-spec/batch-spec.mock.ts
client/web/src/enterprise/batches/batch-spec/execute/backend.ts
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.module.scss
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.story.tsx
client/web/src/enterprise/batches/batch-spec/execute/workspaces/WorkspaceDetails.tsx
cmd/frontend/graphqlbackend/batches.go

@BolajiOlajide BolajiOlajide changed the title batches: fix truncated batch spec output batches: implement pagination for output lines Jan 14, 2023
@github-actions
Copy link
Contributor

Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@BolajiOlajide BolajiOlajide changed the title batches: implement pagination for output lines batches: implement pagination for batch_spec_workspace_steps.output_lines Jan 14, 2023
@BolajiOlajide BolajiOlajide force-pushed the bo/fix-pagination-output-lines branch from a885147 to 5ea5e45 Compare January 14, 2023 16:29
@sg-e2e-regression-test-bob
Copy link

sg-e2e-regression-test-bob commented Jan 14, 2023

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.01% (+2.05 kb) 0.02% (+2.05 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits e854bf3 and b4a20e9 or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@github-actions
Copy link
Contributor

Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@BolajiOlajide BolajiOlajide force-pushed the bo/fix-pagination-output-lines branch from 5ea5e45 to 692daee Compare January 17, 2023 15:35
@github-actions
Copy link
Contributor

Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

"""
A list of output lines.
"""
nodes: [String!]!
Copy link
Contributor

Choose a reason for hiding this comment

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

That's cool, I've always wondered what a connection of scalar data would look like and how you'd query it. Turns out it's that simple! 😄

@BolajiOlajide BolajiOlajide force-pushed the bo/fix-pagination-output-lines branch from db60d13 to 65bd492 Compare January 24, 2023 23:39
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

looks good, I didn't try this out but trust you that you did :)

Copy link
Contributor

@courier-new courier-new left a comment

Choose a reason for hiding this comment

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

Amazing, thank you so much for the refactor here!!

@BolajiOlajide BolajiOlajide force-pushed the bo/fix-pagination-output-lines branch from afccf7b to 48f3f7c Compare January 30, 2023 14:25
@BolajiOlajide BolajiOlajide merged commit b70f530 into main Jan 31, 2023
@BolajiOlajide BolajiOlajide deleted the bo/fix-pagination-output-lines branch January 31, 2023 11:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants