Remove Options on task details fields
Depending on which async action you are triggering, the Details fields of a Task can return different information. See spec.
These field are always returned and are not Optional.
Thus, we should remove the Option. Example:
https://github.com/meilisearch/meilisearch-rust/blob/46d38e61ddf29347b87d59a77e0c69906fb32dbc/src/tasks.rs#L57-L67
Should be changed to
#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct IndexCreation {
pub primary_key: String,
}
#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct IndexUpdate {
pub primary_key: String,
}
These field are always returned and are not
Optional.
According to the doc of indexCreation and indexUpdate, primaryKey can be null in some situations:
primaryKey: Value for the primaryKey field into the POST index payload.nullif no primaryKey has been specified at the time of the index creation.
I wonder if Option<String> was chosen instead of String for such situations?
Hey @choznerol
Definitely Option<string> should be used in this specific situation. My example was not the best 😅
Some others are impacted, for example, based on the specs these one: https://github.com/meilisearch/meilisearch-rust/blob/fc1d41dbb1c03683cf49a44fdb7c9738b8b37d30/src/tasks.rs#L64-L69
and probably others that should be checked.
That’s not an absolute source of truth, but this code might help in meilisearch; it’s how we represent the details on disk. https://github.com/meilisearch/meilisearch/blob/918ce1dd674f99a41545c50272507a275ca53baf/meilisearch-types/src/tasks.rs#L499-L511
Hello,
I had a look at the code and I don't believe this issue is still relevant today. From what I could see, the only place in the code where the Option<> could be removed is TaskType however since TaskType is used for both successedtasks and failedtasks it means there's always a possibility for a Null return so the Option is necessary.
Let me know if I misunderstood anything. Cheers!
That’s true; we would need to duplicate the entire TaskType object or the Succeeded task type into the FailedTask to make something work, and it wouldn’t improve the experience much.
I believe that parts of the issue were fixed in the past, and only the Detail thing remains but isn’t applicable.
Closing, thanks for investigating it @NoodleSamaChan