Skip to content

Commit cd72ccc

Browse files
authored
[CHORE] Clippy for more. (chroma-core#2871)
Left some of worker untouched for concurrent PRs. ## Description of changes *Summarize the changes made by this PR.* - Cleans up warnings. ## Test plan *How are these changes tested?* - [ ] cargo clippy && cargo test w/ tilt up. ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
1 parent 5474965 commit cd72ccc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+1222
-1654
lines changed

rust/benchmark-datasets/src/types.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use anyhow::Result;
2-
use chroma_types::{LogRecord, OperationRecord, UpdateMetadataValue};
32
use rand::prelude::SliceRandom;
43
use serde::{Deserialize, Serialize};
54
use std::{collections::HashMap, future::Future, path::PathBuf, sync::Arc};
@@ -40,8 +39,8 @@ impl<T: RecordDataset> RecordDataset for Arc<T> {
4039
const NAME: &'static str = T::NAME;
4140
const DISPLAY_NAME: &'static str = T::DISPLAY_NAME;
4241

43-
fn init() -> impl Future<Output = Result<Self>> + Send {
44-
async move { Ok(Arc::new(T::init().await?)) }
42+
async fn init() -> Result<Self> {
43+
Ok(Arc::new(T::init().await?))
4544
}
4645

4746
fn create_records_stream(
@@ -202,17 +201,13 @@ mod tests {
202201
const NAME: &'static str = "test";
203202
const DISPLAY_NAME: &'static str = "Test";
204203

205-
fn init() -> impl Future<Output = Result<Self>> + Send {
206-
async move { Ok(TestDataset { records: vec![] }) }
204+
async fn init() -> Result<Self> {
205+
Ok(TestDataset { records: vec![] })
207206
}
208207

209-
fn create_records_stream(
210-
&self,
211-
) -> impl Future<Output = Result<impl Stream<Item = Result<Record>>>> + Send {
212-
async move {
213-
let records = self.records.clone();
214-
Ok(futures::stream::iter(records.into_iter().map(Ok)))
215-
}
208+
async fn create_records_stream(&self) -> Result<impl Stream<Item = Result<Record>>> {
209+
let records = self.records.clone();
210+
Ok(futures::stream::iter(records.into_iter().map(Ok)))
216211
}
217212
}
218213

@@ -221,7 +216,7 @@ mod tests {
221216
#[tokio::test]
222217
async fn test_frozen_query_subset() {
223218
let mut test_dataset = TestDataset::init().await.unwrap();
224-
test_dataset.records = vec!["foo 0", "foo 1", "foo 3", "bar 0", "bar 2"]
219+
test_dataset.records = ["foo 0", "foo 1", "foo 3", "bar 0", "bar 2"]
225220
.iter()
226221
.map(|&content| Record {
227222
document: content.to_string(),
@@ -230,7 +225,7 @@ mod tests {
230225
.collect();
231226

232227
let mut test_query_dataset = TestDataset::init().await.unwrap();
233-
test_query_dataset.records = vec!["foo", "bar", "baz"]
228+
test_query_dataset.records = ["foo", "bar", "baz"]
234229
.iter()
235230
.map(|&content| Record {
236231
document: content.to_string(),

rust/index/benches/dataset_utilities.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,15 @@ pub async fn get_record_dataset<RecordCorpus: RecordDataset>() -> RecordCorpus {
1818
.with_style(style.clone());
1919
record_corpus_spinner.enable_steady_tick(Duration::from_millis(50));
2020

21-
let record_corpus = RecordCorpus::init()
21+
RecordCorpus::init()
2222
.and_then(|r| async {
2323
record_corpus_spinner.set_style(finish_style.clone());
2424
record_corpus_spinner.set_prefix("✔︎");
2525
record_corpus_spinner.finish_and_clear();
2626
Ok(r)
2727
})
2828
.await
29-
.expect("Failed to initialize record corpus");
30-
31-
record_corpus
29+
.expect("Failed to initialize record corpus")
3230
}
3331

3432
pub async fn get_record_query_dataset_pair<

rust/index/benches/full_text.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ fn bench_querying(c: &mut Criterion) {
170170
.await
171171
.unwrap();
172172

173-
assert!(result.len() > 0, "Query result is empty");
173+
assert!(!result.is_empty(), "Query result is empty");
174174
},
175175
criterion::BatchSize::SmallInput,
176176
)

rust/index/src/hnsw_provider.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ impl HnswIndexProvider {
416416
/// Purge entries from the cache by index ID and remove temporary files from disk.
417417
pub async fn purge_by_id(&mut self, index_ids: &[Uuid]) {
418418
for index_id in index_ids {
419-
match self.remove_temporary_files(&index_id).await {
419+
match self.remove_temporary_files(index_id).await {
420420
Ok(_) => {}
421421
Err(e) => {
422422
tracing::error!("Failed to remove temporary files: {}", e);

rust/index/src/utils.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#[cfg(test)]
22
use rand::Rng;
33

4+
// This is not reserved for testing. If you need to use it outside test contexts, remove this
5+
// line. It exists solely to satisfy the linter.
46
#[cfg(test)]
57
pub(super) fn generate_random_data(n: usize, d: usize) -> Vec<f32> {
68
let mut rng: rand::prelude::ThreadRng = rand::thread_rng();

rust/worker/src/assignment/assignment_policy.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,15 @@ pub(crate) struct RendezvousHashingAssignmentPolicy {
3737
members: Vec<String>,
3838
}
3939

40+
// This is not reserved for testing. If you need to use it outside test contexts, remove this
41+
// line. It exists solely to satisfy the linter.
42+
#[cfg(test)]
4043
impl RendezvousHashingAssignmentPolicy {
4144
pub(crate) fn new() -> RendezvousHashingAssignmentPolicy {
42-
return RendezvousHashingAssignmentPolicy {
45+
RendezvousHashingAssignmentPolicy {
4346
hasher: Murmur3Hasher {},
4447
members: vec![],
45-
};
48+
}
4649
}
4750
}
4851

@@ -51,14 +54,12 @@ impl Configurable<AssignmentPolicyConfig> for RendezvousHashingAssignmentPolicy
5154
async fn try_from_config(
5255
config: &AssignmentPolicyConfig,
5356
) -> Result<Self, Box<dyn ChromaError>> {
54-
let assignment_policy_config = match config {
55-
AssignmentPolicyConfig::RendezvousHashing(config) => config,
56-
};
57+
let AssignmentPolicyConfig::RendezvousHashing(assignment_policy_config) = config;
5758
let hasher = match assignment_policy_config.hasher {
5859
HasherType::Murmur3 => Murmur3Hasher {},
5960
};
6061
return Ok(RendezvousHashingAssignmentPolicy {
61-
hasher: hasher,
62+
hasher,
6263
members: vec![],
6364
});
6465
}
@@ -73,7 +74,7 @@ impl AssignmentPolicy for RendezvousHashingAssignmentPolicy {
7374
fn get_members(&self) -> Vec<String> {
7475
// This is not designed to be used frequently for now, nor is the number of members
7576
// expected to be large, so we can just clone the members
76-
return self.members.clone();
77+
self.members.clone()
7778
}
7879

7980
fn set_members(&mut self, members: Vec<String>) {

rust/worker/src/assignment/rendezvous_hash.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ pub(crate) fn assign<H: Hasher>(
8383

8484
match max_member {
8585
Some(max_member) => return Ok(max_member.as_ref().to_string()),
86-
None => return Err(AssignmentError::NoMembers),
86+
None => Err(AssignmentError::NoMembers),
8787
}
8888
}
8989

@@ -109,10 +109,10 @@ impl Hasher for Murmur3Hasher {
109109
let member_hash_64 = member_hash as u64;
110110
let key_hash_64 = key_hash as u64;
111111
let merged = merge_hashes(member_hash_64, key_hash_64);
112-
return Ok(merged);
112+
Ok(merged)
113113
}
114-
_ => return Err(AssignmentError::HashError),
115-
};
114+
_ => Err(AssignmentError::HashError),
115+
}
116116
}
117117
}
118118

@@ -164,8 +164,7 @@ mod tests {
164164
}
165165

166166
let expected = num_keys / member_count;
167-
for i in 0..member_count {
168-
let count = counts[i];
167+
for count in counts.iter().take(member_count).copied() {
169168
let diff = count - expected as i32;
170169
assert!(diff.abs() < tolerance);
171170
}

rust/worker/src/compactor/compaction_manager.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub(crate) struct CompactionManager {
3737
// Dependencies
3838
log: Box<Log>,
3939
sysdb: Box<SysDb>,
40+
#[allow(dead_code)]
4041
storage: Storage,
4142
blockfile_provider: BlockfileProvider,
4243
hnsw_index_provider: HnswIndexProvider,
@@ -45,6 +46,7 @@ pub(crate) struct CompactionManager {
4546
// Config
4647
compaction_manager_queue_size: usize,
4748
compaction_interval: Duration,
49+
#[allow(dead_code)]
4850
min_compaction_size: usize,
4951
max_compaction_size: usize,
5052
max_partition_size: usize,
@@ -65,6 +67,7 @@ impl ChromaError for CompactionError {
6567
}
6668

6769
impl CompactionManager {
70+
#[allow(clippy::too_many_arguments)]
6871
pub(crate) fn new(
6972
scheduler: Scheduler,
7073
log: Box<Log>,
@@ -344,7 +347,7 @@ mod tests {
344347
#[tokio::test]
345348
async fn test_compaction_manager() {
346349
let mut log = Box::new(Log::InMemory(InMemoryLog::new()));
347-
let mut in_memory_log = match *log {
350+
let in_memory_log = match *log {
348351
Log::InMemory(ref mut log) => log,
349352
_ => panic!("Expected InMemoryLog"),
350353
};
@@ -353,9 +356,9 @@ mod tests {
353356

354357
let collection_uuid_1 = Uuid::from_str("00000000-0000-0000-0000-000000000001").unwrap();
355358
in_memory_log.add_log(
356-
collection_uuid_1.clone(),
357-
Box::new(InternalLogRecord {
358-
collection_id: collection_uuid_1.clone(),
359+
collection_uuid_1,
360+
InternalLogRecord {
361+
collection_id: collection_uuid_1,
359362
log_offset: 0,
360363
log_ts: 1,
361364
record: LogRecord {
@@ -369,14 +372,14 @@ mod tests {
369372
operation: Operation::Add,
370373
},
371374
},
372-
}),
375+
},
373376
);
374377

375378
let collection_uuid_2 = Uuid::from_str("00000000-0000-0000-0000-000000000002").unwrap();
376379
in_memory_log.add_log(
377-
collection_uuid_2.clone(),
378-
Box::new(InternalLogRecord {
379-
collection_id: collection_uuid_2.clone(),
380+
collection_uuid_2,
381+
InternalLogRecord {
382+
collection_id: collection_uuid_2,
380383
log_offset: 0,
381384
log_ts: 2,
382385
record: LogRecord {
@@ -390,7 +393,7 @@ mod tests {
390393
operation: Operation::Add,
391394
},
392395
},
393-
}),
396+
},
394397
);
395398

396399
let mut sysdb = Box::new(SysDb::Test(TestSysDb::new()));

0 commit comments

Comments
 (0)