medschool: validate json snippets in config files #2630
Conversation
| jsonschema = "0.18.0" | ||
| serde_json = "1.0.122" |
There was a problem hiding this comment.
| jsonschema = "0.18.0" | |
| serde_json = "1.0.122" | |
| jsonschema = "0.18" | |
| serde_json = "1.0" |
We usually just use major.min everywhere else right?
| let schema = if let Some(schema) = schema { | ||
| let schema = fs::read_to_string(schema)?; | ||
| let schema: Value = | ||
| serde_json::from_str(&schema).map_err(|e| DocsError::Json(e, schema))?; | ||
| let compiled_schema = | ||
| JSONSchema::compile(&schema).map_err(|e| DocsError::JsonSchema(e.to_string()))?; | ||
| Some(compiled_schema) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
if let Some {} else None is just .map.
| @@ -200,43 +203,53 @@ pub fn parse_docs_into_set<'a>( | |||
| /// DFS helper function to resolve the references of the types. Returns the docs of the fields of | |||
| /// the field we're currently looking at search till its leaf nodes. The leaf here means a primitive | |||
| /// type for which we don't have a [`PartialType`]. | |||
| #[tracing::instrument(level = "trace", ret)] | |||
There was a problem hiding this comment.
| #[tracing::instrument(level = "trace", ret)] | |
| #[tracing::instrument(level = Level::TRACE, ret)] |
For some reason we started doing it like this now.
| @@ -279,48 +292,59 @@ fn dfs_fields<'a, const MAX_RECURSION_LEVEL: usize>( | |||
| /// be where we resolve all references and return the same HashSet and let the caller | |||
| /// decide what the root should be. | |||
| #[tracing::instrument(level = "trace", ret)] | |||
There was a problem hiding this comment.
| #[tracing::instrument(level = "trace", ret)] | |
| #[tracing::instrument(level = Level::TRACE, ret)] |
Also change it here, since you're touching this function.
| types: HashSet<PartialType>, | ||
| schema: Option<JSONSchema>, | ||
| ) -> Result<Option<PartialType>, DocsError> { | ||
| // Maximum recursion level for safety. |
There was a problem hiding this comment.
| // Maximum recursion level for safety. | |
| /// Maximum recursion level for safety. |
| .inspect_err(|e| error!(error = ?e, context=?field, "Error validating JSON blocks"))?; | ||
| } | ||
| // get the type of the field from the types set to recurse into it's fields | ||
| match types.get(&field.ty) { |
There was a problem hiding this comment.
I don't think you need to change this so much just due to the function returning a Result now?
The logic inside this match looks very similar to the iterator (at a glance at least).
| const MAX_RECURSION_LEVEL: usize = 10; | ||
| // Cache to perform memoization between recursive calls so we don't have to resolve the same | ||
| // type multiple times. Mapping between `ident` -> `resolved_docs`. | ||
| // For example, if we have a types [`A`, `B`, `C`] and A has a field of type `B` and `B` has a | ||
| // field of type `C`, and `C` has already been resolved, we don't want to resolve `C` again | ||
| // as we iterate over the types. A -> (B -> C), (B -> C), (C) | ||
| let mut cache = HashMap::with_capacity(types.len()); | ||
| let mut max_area = 0; |
There was a problem hiding this comment.
Same thing here, the logic looks pretty much the same, except it's more Result-y now? You could use try_collect for example.
of course bhai, the whole closure/functional thing made the code a mess to be honest, its much easier and clear to read it with a for loop |
Aw, I thought it would be just a matter of changing some |
hi, coming back to this, id like to keep the loop there just to make it more readable! will address the comments! sorry for the delay |
Alright! I'm not a
No worries bhai! After your previous fix the tool is already working way better. |
|
Hey @infiniteregrets, is there any work in progress on this one? |
|
Seems like the PR is dead ☠️ |
closes #2525