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

Bulk insert of failed receipts using query macro #329

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

taslimmuhammed
Copy link
Contributor

Fixed #273
Added error logs storing to the table
Successfully ran commands:-
cargo clippy --all-targets --all-features
cargo fmt
cargo sqlx prepare --workspace -- --all-targets
please do recheck on your device

Copy link
Collaborator

@gusinacio gusinacio left a comment

Choose a reason for hiding this comment

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

I think this is the last review. It has been a great work. Looking forward to fix these comments and merge it.

One suggestion: don't close and open new PRs unless they are have totally different code from each other. I see that you created PRs #326, #327, #328 and now #329 but they are all pointing to the same branch with one extra commit in each of them. It's hard to keep track of suggestions in other PRs.

migrations/20230915230734_tap_ravs.down.sql Outdated Show resolved Hide resolved
tap-agent/src/agent/sender_allocation.rs Outdated Show resolved Hide resolved
@taslimmuhammed
Copy link
Contributor Author

Please take a look at the latest commit, hope everything's good

@coveralls
Copy link

Pull Request Test Coverage Report for Build 11119033601

Details

  • 29 of 33 (87.88%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 68.296%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tap-agent/src/agent/sender_allocation.rs 29 33 87.88%
Files with Coverage Reduction New Missed Lines %
tap-agent/src/agent/sender_allocation.rs 1 89.74%
Totals Coverage Status
Change from base Build 10967317486: 0.03%
Covered Lines: 4233
Relevant Lines: 6198

💛 - Coveralls

Copy link
Collaborator

@gusinacio gusinacio left a comment

Choose a reason for hiding this comment

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

This all looks good to me. Could you do a rebase on your side to update the commit messages so it passes our CI? This is a guide on how to do your commit messages.

@gusinacio
Copy link
Collaborator

Actually, I can do that on my side. Thank you for the great work and thanks for your contribution!

@taslimmuhammed
Copy link
Contributor Author

Thanks a lot for the support sir, hoping to contribute more to the project

@gusinacio gusinacio merged commit f65d95c into graphprotocol:main Oct 1, 2024
7 of 10 checks passed
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.

[Feat.Req] Bulk insert of failed receipts
4 participants