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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: add limit to receipts retrieve #190

Merged
merged 1 commit into from Nov 28, 2023

Conversation

aasseman
Copy link
Collaborator

Helps with keeping the rule of 15,000 max receipts per aggregation request.
Also adds a helper function to implement the limit safely.

Note about tap_core/Cargo.toml:

I started getting

error[E0599]: no function or associated item named `new` found for struct `tokio::runtime::Runtime` in the current scope
  --> tap_core/benches/timeline_aggretion_protocol_benchmark.rs:49:34
   |
49 |     let async_runtime = Runtime::new().unwrap();
   |                                  ^^^ function or associated item not found in `Runtime`

Not sure why it even started happening, but the fix was to add the rt-multi-thread feature to tokio 馃し

@aasseman aasseman added the enhancement New feature or request label Nov 15, 2023
@aasseman aasseman self-assigned this Nov 15, 2023

This comment has been minimized.

@aasseman aasseman force-pushed the aasseman/feat_receipts_retrieve_limit branch from 9078cbf to d25b615 Compare November 15, 2023 20:25

This comment has been minimized.

Copy link

github-actions bot commented Nov 15, 2023

Pull Request Test Coverage Report for Build 7022943430

  • 48 of 54 (88.89%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 82.555%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tap_core/src/adapters/receipt_storage_adapter.rs 31 34 91.18%
tap_core/src/adapters/test/receipt_storage_adapter_mock.rs 7 10 70.0%
Totals Coverage Status
Change from base Build 7013080532: 0.2%
Covered Lines: 1273
Relevant Lines: 1542

馃挍 - Coveralls

@aasseman aasseman force-pushed the aasseman/feat_receipts_retrieve_limit branch from d25b615 to a6289e8 Compare November 24, 2023 00:06
@aasseman
Copy link
Collaborator Author

Rebased onto latest main

This comment has been minimized.

Copy link
Contributor

@ColePBryan ColePBryan 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. The one change we might consider is to make it possible to know programmatically that the list is incomplete. I'm not what the best approach would be for accomplishing this. I'm setting to approve since the comment makes it very clear how it works.

Helps with keeping the rule of 15,000 max receipts per aggregation
request.
Also adds a helper function to implement the limit safely.

Signed-off-by: Alexis Asseman <alexis@semiotic.ai>
@aasseman aasseman force-pushed the aasseman/feat_receipts_retrieve_limit branch from a6289e8 to 0ce2aab Compare November 28, 2023 18:26
@aasseman
Copy link
Collaborator Author

Rebased onto latest main

Copy link

馃 Cargo Audit Report 馃

(Empty means OK! 馃憤)

@aasseman aasseman merged commit 4f55f48 into main Nov 28, 2023
7 checks passed
@aasseman aasseman deleted the aasseman/feat_receipts_retrieve_limit branch November 28, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants