Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resume with blockchain progress endpoints #17

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

vegarsti
Copy link
Member

@vegarsti vegarsti commented Jun 11, 2024

This PR adds use of the GetBlockchainProgress endpoint on startup, and continuous calls to PostProgressEndpoint to update the progress.

Try with

export DUNE_API_KEY=<dune team api key in dev>
go run cmd/main.go --blockchain-name byob --dune-api-url "https://api.dev.dune.com" --rpc-node-url "https://sepolia.rpc.zora.energy"

Towards CHAIN-1588.

Copy link
Member Author

vegarsti commented Jun 11, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vegarsti and the rest of your teammates on Graphite Graphite

@vegarsti vegarsti force-pushed the use-blockchain-progress branch 2 times, most recently from 37cf797 to a2d65bf Compare June 12, 2024 07:24
@vegarsti vegarsti marked this pull request as ready for review June 12, 2024 07:24
@vegarsti vegarsti force-pushed the progress-report branch 2 times, most recently from 6fc3ab3 to d28f60a Compare June 12, 2024 07:30
@vegarsti vegarsti changed the title Use blockchain progress in main Use blockchain progress endpoints to resume Jun 12, 2024
@vegarsti vegarsti force-pushed the use-blockchain-progress branch 2 times, most recently from 7c5672e to 5b1d3c5 Compare June 12, 2024 07:41
Comment on lines 106 to 113
return ctx.Err()
return nil
Copy link
Member Author

Choose a reason for hiding this comment

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

It's not an error if the context is canceled

Copy link
Contributor

Choose a reason for hiding this comment

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

but we rely on this to stop the other threads I think

ingester/mainloop.go Outdated Show resolved Hide resolved
ingester/mainloop_test.go Outdated Show resolved Hide resolved
ingester/mainloop_test.go Outdated Show resolved Hide resolved
vegarsti added a commit that referenced this pull request Jun 12, 2024
This PR adds Get and Post methods for the progress report API
(duneanalytics/duneapi#559).

We use the endpoints in #17 (and they work!).

Towards CHAIN-1588.
Base automatically changed from progress-report to main June 12, 2024 08:18
@vegarsti vegarsti requested review from msf and helanto and removed request for msf June 12, 2024 08:19
@vegarsti vegarsti changed the title Use blockchain progress endpoints to resume Resume with blockchain progress endpoints Jun 12, 2024
Copy link
Contributor

@helanto helanto left a comment

Choose a reason for hiding this comment

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

Some questions

ingester/ingester.go Outdated Show resolved Hide resolved
ingester/mainloop.go Show resolved Hide resolved
ingester/mainloop.go Show resolved Hide resolved
ingester/ingester.go Outdated Show resolved Hide resolved
if err != nil {
return errors.Errorf("failed to get latest block number: %w", err)
}
progress, err := i.dune.GetProgressReport(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not:

if startBlockNumber < 0 {
   progress, err := i.dune.GetProgressReport(ctx)
   ...
   startBlockNumber = progressReport.LastIngestedBlockNumber + 1
}

Copy link
Member Author

Choose a reason for hiding this comment

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

aha yes, that's a good idea

Comment on lines 106 to 113
return ctx.Err()
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

but we rely on this to stop the other threads I think

ingester/mainloop.go Show resolved Hide resolved
ingester/mainloop.go Show resolved Hide resolved
ingester/mainloop_test.go Outdated Show resolved Hide resolved
ingester/mainloop_test.go Outdated Show resolved Hide resolved
@vegarsti vegarsti force-pushed the use-blockchain-progress branch 2 times, most recently from 912364b to d16a777 Compare June 12, 2024 09:12
@vegarsti
Copy link
Member Author

Whew, time for a coffee

@vegarsti vegarsti merged commit 7b88c35 into main Jun 12, 2024
1 check passed
@vegarsti vegarsti deleted the use-blockchain-progress branch June 12, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants