-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add bulk_insert option to add list of records using auditor client and pyauditor #582
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #582 +/- ##
==========================================
+ Coverage 64.06% 64.47% +0.41%
==========================================
Files 46 46
Lines 5387 5531 +144
Branches 58 58
==========================================
+ Hits 3451 3566 +115
- Misses 1936 1965 +29
|
74ad7ac
to
1598419
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! I only have a few minor comments, mostly related to the documetation, see below.
auditor/src/lib.rs
Outdated
//! # let records: Result<Vec<RecordAdd>, _> = (0..5) | ||
//! # .map(|i| RecordAdd::new(&format!("record-{}", i), HashMap::new(), vec![], start_time)) | ||
//! # .collect(); | ||
//! client.bulk_insert(&records?).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can records
be of type Vec<RecordAdd>
so that we then have client.bulk_insert(&records).await?;
inside the docs?
auditor/src/routes/add.rs
Outdated
#[tracing::instrument(name = "Adding a record to the database", skip(records, pool))] | ||
pub async fn bulk_add( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tracing instrument name doesn't match what the function is doing. Maybe switch to "Adding multiple records to the database" or something like this.
pyauditor/docs/examples.rst
Outdated
@@ -98,6 +98,14 @@ Assuming that a record and a client were already created, the record can be push | |||
|
|||
await client.add(record) | |||
|
|||
Pushing list of records to Auditor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing list -> Pushing a list
pyauditor/src/blocking_client.rs
Outdated
@@ -121,6 +121,19 @@ impl AuditorClientBlocking { | |||
.map_err(|e| pyo3::exceptions::PyRuntimeError::new_err(format!("{e}"))) | |||
} | |||
|
|||
/// add(record: Record) | |||
/// Push a record to the Auditor instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above (a record != bulk_insert)
pyauditor/src/client.rs
Outdated
@@ -797,6 +797,25 @@ impl AuditorClient { | |||
}) | |||
} | |||
|
|||
/// add(record: Record) | |||
/// Push a record to the Auditor instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
57431ca
to
06ebd00
Compare
06ebd00
to
980b0ef
Compare
This closes #580
Bulk insertion of records is implemented for auditor. This allows list of records to be passed as an argument to auditor client and pyauditor methods.
Added unit test, API tests for auditor and pyauditor client.
Created new end point called /records - this will be used to perform bulk operations (in the future, this can be extended to update as well using PUT method)
Written doc strings and tutorial to use bulk_insert functionality.
Added CHANGELOG.md