Repository: qdrant/qdrant
File: lib/shard/src/snapshots/snapshot_manifest.rs (Rust)

Code before:
use std::collections::{HashMap, HashSet};
use std::path::Path;

use fs_err as fs;
use fs_err::File;
use segment::common::operation_error::{OperationError, OperationResult};
use segment::data_types::manifest::SegmentManifest;
use segment::segment::snapshot::SEGMENT_MANIFEST_FILE_NAME;
use segment::types::SeqNumberType;

use crate::files::segments_path;

#[derive(Clone, Debug, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
#[serde(transparent)]
pub struct SnapshotManifest {
    segments: HashMap<String, SegmentManifest>,
}

impl SnapshotManifest {
    /// Create an empty snapshot manifest
    pub fn empty() -> Self {
        Self {
            segments: HashMap::new(),
        }
    }

    /// Creates a `SnapshotManifest` by looking at the files of the unpacked snapshot
    ///
    /// # Arguments
    /// * `snapshot_path` - Path to the unpacked snapshot directory
    pub fn load_from_snapshot(
        snapshot_path: &Path,
        recovery_type: Option<RecoveryType>,
    ) -> OperationResult<Self> {
        let segments_path = segments_path(snapshot_path);

        let mut snapshot_segments = HashSet::new();
        let mut snapshot_manifest = SnapshotManifest::default();

        for segment_entry in fs::read_dir(segments_path)? {
            let segment_path = segment_entry?.path();

            if !segment_path.is_dir() {
                log::warn!(
                    "segment path {} in extracted snapshot {} is not a directory",
                    segment_path.display(),
                    snapshot_path.display(),
                );

                continue;

Review comment (suggestion):
This is equal to `SnapshotManifest::default()`. I suggest to only keep one of them, where `default()` would be more conventional.

Code after:
use std::collections::{HashMap, HashSet};
use std::path::Path;

use fs_err as fs;
use fs_err::File;
use segment::common::operation_error::{OperationError, OperationResult};
use segment::data_types::manifest::SegmentManifest;
use segment::segment::snapshot::SEGMENT_MANIFEST_FILE_NAME;
use segment::types::SeqNumberType;

use crate::files::segments_path;

#[derive(Clone, Debug, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
#[serde(transparent)]
pub struct SnapshotManifest {
    segments: HashMap<String, SegmentManifest>,
}

impl SnapshotManifest {
    /// Creates a `SnapshotManifest` by looking at the files of the unpacked snapshot
    ///
    /// # Arguments
    /// * `snapshot_path` - Path to the unpacked snapshot directory
    pub fn load_from_snapshot(
        snapshot_path: &Path,
        recovery_type: Option<RecoveryType>,
    ) -> OperationResult<Self> {
        let segments_path = segments_path(snapshot_path);

        let mut snapshot_segments = HashSet::new();
        let mut snapshot_manifest = SnapshotManifest::default();

        for segment_entry in fs::read_dir(segments_path)? {
            let segment_path = segment_entry?.path();

            if !segment_path.is_dir() {
                log::warn!(
                    "segment path {} in extracted snapshot {} is not a directory",
                    segment_path.display(),
                    snapshot_path.display(),
                );

                continue;
            }

            let segment_id = segment_path
                .file_name()
                .and_then(|segment_id| segment_id.to_str())
                .expect("segment path ends with a valid segment id");