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

refactor file writing#54673

Merged
varungandhi-src merged 1 commit into
vg/stream-indexesfrom
vg+ef/stream-indexes
Jul 12, 2023
Merged

refactor file writing#54673
varungandhi-src merged 1 commit into
vg/stream-indexesfrom
vg+ef/stream-indexes

Conversation

@efritz

@efritz efritz commented Jul 6, 2023

Copy link
Copy Markdown
Contributor

cc @varungandhi-src Re-commented the flow and refactored so that we favor early-exits over global state (buf/indexReader being nil-able was confusing to determine what states were valid). I think the only behavioral change is we hard-error on the creation of a temp file instead of logging+doing our best. We can preserve that old behavior if it was an intentional design decision.

@efritz efritz force-pushed the vg+ef/stream-indexes branch from 7530c69 to e46c546 Compare July 6, 2023 12:30
@sourcegraph-bot

sourcegraph-bot commented Jul 6, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 6f99e7d...e46c546.

Notify File(s)
@Strum355 internal/codeintel/uploads/internal/background/processor/job_worker_handler.go

}()

// Wrap the file reader
indexReader, err := newGzipReadSeeker(tempFile)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAICT, we're missing a write to the file in the first place... so we won't be able to read anything here.

@varungandhi-src

Copy link
Copy Markdown
Contributor

I'm going to merge this PR and make some smaller tweaks in my PR.

@varungandhi-src varungandhi-src merged commit ec6184b into vg/stream-indexes Jul 12, 2023
@varungandhi-src varungandhi-src deleted the vg+ef/stream-indexes branch July 12, 2023 00:46
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.

3 participants