Skip to content

Commit 930942e

Browse files
parmesantparmesant
committed
Coderabbit suggestions
1 parent ad1e884 commit 930942e

File tree

13 files changed

+70
-85
lines changed

13 files changed

+70
-85
lines changed

‎src/alerts/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -995,7 +995,7 @@ impl AlertManagerTrait for Alerts {
995995
let mut map = self.alerts.write().await;
996996

997997
// Get alerts path and read raw bytes for migration handling
998-
let raw_objects = PARSEABLE.metastore.get_alerts().await.unwrap_or_default();
998+
let raw_objects = PARSEABLE.metastore.get_alerts().await?;
999999

10001000
for raw_bytes in raw_objects {
10011001
// First, try to parse as JSON Value to check version

‎src/alerts/target.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,19 @@ pub struct TargetConfigs {
5656
impl TargetConfigs {
5757
/// Loads alerts from disk, blocks
5858
pub async fn load(&self) -> anyhow::Result<()> {
59+
let targets = PARSEABLE.metastore.get_targets().await?;
5960
let mut map = self.target_configs.write().await;
60-
61-
for target in PARSEABLE.metastore.get_targets().await.unwrap_or_default() {
61+
for target in targets {
6262
map.insert(target.id, target);
6363
}
6464

6565
Ok(())
6666
}
6767

6868
pub async fn update(&self, target: Target) -> Result<(), AlertError> {
69+
PARSEABLE.metastore.put_target(&target).await?;
6970
let mut map = self.target_configs.write().await;
7071
map.insert(target.id, target.clone());
71-
PARSEABLE.metastore.put_target(&target).await?;
7272
Ok(())
7373
}
7474

‎src/handlers/http/alerts.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,15 @@ pub async fn post(
208208

209209
alert.validate(&session_key).await?;
210210

211-
// now that we've validated that the user can run this query
212-
// move on to saving the alert in ObjectStore
213-
alerts.update(alert).await;
214-
211+
// update persistent storage first
215212
PARSEABLE
216213
.metastore
217214
.put_alert(&alert.to_alert_config())
218215
.await?;
219216

217+
// update in memory
218+
alerts.update(alert).await;
219+
220220
// start the task
221221
alerts.start_task(alert.clone_box()).await?;
222222

‎src/handlers/http/modal/utils/rbac_utils.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub async fn get_metadata() -> Result<crate::storage::StorageMetadata, ObjectSto
2727
.get_parseable_metadata()
2828
.await
2929
.map_err(|e| ObjectStorageError::MetastoreError(Box::new(e.to_detail())))?
30-
.expect("metadata is initialized");
30+
.ok_or_else(|| ObjectStorageError::Custom("parseable metadata not initialized".into()))?;
3131
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
3232
}
3333

‎src/handlers/http/oidc.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ async fn get_metadata() -> Result<crate::storage::StorageMetadata, ObjectStorage
448448
.get_parseable_metadata()
449449
.await
450450
.map_err(|e| ObjectStorageError::MetastoreError(Box::new(e.to_detail())))?
451-
.expect("metadata is initialized");
451+
.ok_or_else(|| ObjectStorageError::Custom("parseable metadata not initialized".into()))?;
452452
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
453453
}
454454

‎src/handlers/http/role.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async fn get_metadata() -> Result<crate::storage::StorageMetadata, ObjectStorage
146146
.get_parseable_metadata()
147147
.await
148148
.map_err(|e| ObjectStorageError::MetastoreError(Box::new(e.to_detail())))?
149-
.expect("metadata is initialized");
149+
.ok_or_else(|| ObjectStorageError::Custom("parseable metadata not initialized".into()))?;
150150
Ok(serde_json::from_slice::<StorageMetadata>(&metadata)?)
151151
}
152152

‎src/metastore/metastore_traits.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ use crate::{
3636
#[async_trait]
3737
pub trait Metastore: std::fmt::Debug + Send + Sync {
3838
async fn initiate_connection(&self) -> Result<(), MetastoreError>;
39-
async fn list_objects(&self) -> Result<(), MetastoreError>;
40-
async fn get_object(&self) -> Result<(), MetastoreError>;
4139
async fn get_objects(&self, parent_path: &str) -> Result<Vec<Bytes>, MetastoreError>;
4240

4341
/// alerts

‎src/metastore/metastores/object_store_metastore.rs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,6 @@ impl Metastore for ObjectStoreMetastore {
6868
unimplemented!()
6969
}
7070

71-
/// Might implement later
72-
async fn list_objects(&self) -> Result<(), MetastoreError> {
73-
unimplemented!()
74-
}
75-
76-
/// Might implement later
77-
async fn get_object(&self) -> Result<(), MetastoreError> {
78-
unimplemented!()
79-
}
80-
8171
/// Fetch mutiple .json objects
8272
async fn get_objects(&self, parent_path: &str) -> Result<Vec<Bytes>, MetastoreError> {
8373
Ok(self
@@ -105,7 +95,12 @@ impl Metastore for ObjectStoreMetastore {
10595

10696
/// This function puts an alert in the object store at the given path
10797
async fn put_alert(&self, obj: &dyn MetastoreObject) -> Result<(), MetastoreError> {
108-
let path = alert_json_path(Ulid::from_string(&obj.get_object_id()).unwrap());
98+
let id = Ulid::from_string(&obj.get_object_id()).map_err(|e| MetastoreError::Error {
99+
status_code: StatusCode::BAD_REQUEST,
100+
message: e.to_string(),
101+
flow: "put_alert".into(),
102+
})?;
103+
let path = alert_json_path(id);
109104

110105
Ok(self.storage.put_object(&path, to_bytes(obj)).await?)
111106
}
@@ -536,7 +531,10 @@ impl Metastore for ObjectStoreMetastore {
536531
.await?
537532
.iter()
538533
// we should be able to unwrap as we know the data is valid schema
539-
.map(|byte_obj| serde_json::from_slice(byte_obj).expect("data is valid json"))
534+
.map(|byte_obj| {
535+
serde_json::from_slice(byte_obj)
536+
.unwrap_or_else(|_| panic!("got an invalid schema for stream: {stream_name}"))
537+
})
540538
.collect())
541539
}
542540

@@ -661,6 +659,7 @@ impl Metastore for ObjectStoreMetastore {
661659
.await
662660
.map_err(MetastoreError::ObjectStorageError)
663661
} else {
662+
// not local-disk, object storage
664663
let mut result_file_list = HashSet::new();
665664
let resp = self.storage.list_with_delimiter(None).await?;
666665

@@ -669,7 +668,12 @@ impl Metastore for ObjectStoreMetastore {
669668
.iter()
670669
.flat_map(|path| path.parts())
671670
.map(|name| name.as_ref().to_string())
672-
.filter(|name| name != PARSEABLE_ROOT_DIRECTORY && name != USERS_ROOT_DIR)
671+
.filter(|name| {
672+
name != PARSEABLE_ROOT_DIRECTORY
673+
&& name != USERS_ROOT_DIR
674+
&& name != SETTINGS_ROOT_DIRECTORY
675+
&& name != ALERTS_ROOT_DIRECTORY
676+
})
673677
.collect::<Vec<_>>();
674678

675679
for stream in streams {

‎src/metastore/mod.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl MetastoreError {
9595
file_path: None,
9696
timestamp: Some(chrono::Utc::now()),
9797
metadata: std::collections::HashMap::new(),
98-
status_code: 500,
98+
status_code: 400,
9999
},
100100
MetastoreError::JsonSchemaError { message } => MetastoreErrorDetail {
101101
operation: "JsonSchemaError".to_string(),
@@ -104,7 +104,7 @@ impl MetastoreError {
104104
file_path: None,
105105
timestamp: Some(chrono::Utc::now()),
106106
metadata: std::collections::HashMap::new(),
107-
status_code: 500,
107+
status_code: 400,
108108
},
109109
MetastoreError::InvalidJsonStructure { expected, found } => MetastoreErrorDetail {
110110
operation: "InvalidJsonStructure".to_string(),
@@ -118,7 +118,7 @@ impl MetastoreError {
118118
]
119119
.into_iter()
120120
.collect(),
121-
status_code: 500,
121+
status_code: 400,
122122
},
123123
MetastoreError::MissingJsonField { field } => MetastoreErrorDetail {
124124
operation: "MissingJsonField".to_string(),
@@ -127,7 +127,7 @@ impl MetastoreError {
127127
file_path: None,
128128
timestamp: Some(chrono::Utc::now()),
129129
metadata: [("field".to_string(), field.clone())].into_iter().collect(),
130-
status_code: 500,
130+
status_code: 400,
131131
},
132132
MetastoreError::InvalidJsonValue { field, reason } => MetastoreErrorDetail {
133133
operation: "InvalidJsonValue".to_string(),
@@ -141,7 +141,7 @@ impl MetastoreError {
141141
]
142142
.into_iter()
143143
.collect(),
144-
status_code: 500,
144+
status_code: 400,
145145
},
146146
}
147147
}

‎src/parseable/streams.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ impl Stream {
660660
return Ok(None);
661661
}
662662

663-
Ok(Some(Schema::try_merge(schemas).unwrap()))
663+
Ok(Some(Schema::try_merge(schemas)?))
664664
}
665665

666666
fn write_parquet_part_file(

0 commit comments

Comments
 (0)