Skip to content

Send documents as jsonlines for POST and PUT requests #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Send documents as jsonlines for POST and PUT requests #306

wants to merge 2 commits into from

Conversation

matthias-wright
Copy link

This PR addresses #265.

The main obstacle I see is that serde_json does not offer a way to serialize data to jsonlines. For example, vectors are automatically serialized to json arrays by serde_json.to_string().
A simple workaround is to iterate over the collection and serialize each item individually. Then we can join them using \n as the separator to obtain jsonlines:

let movies = vec![Movie { id: 1, name: String::from("Life of Pi") },
                  Movie { id: 2, name: String::from("Mad Max") },
                  Movie { id: 3, name: String::from("Moana") }];

let mut jsonlines = movies.iter().map(|x| serde_json::to_string(x).unwrap()).collect::<Vec<String>>().join("\n");
jsonlines.push('\n');

However, in the request function, where the data is serialized, Serialize is the only trait bound for the data (Input). That means we cannot iterate over the data to obtain jsonlines as in the example above.

To work around this we could create another request function (and call it request_jsonlines or whatever) which only takes in data that is iterable. Then we would also have to create a new Method enum. I did not want to do this because it would entail a lot of duplicate code.
Another option would be to write a function that converts a json array to jsonlines. However, that would probably be less efficient than computing the jsonlines directly (as shown above).
Hence, I decided to adapt the Method enum, such that it can hold two different types. A type that is iterable (for jsonlines) and a type that is not necessary iterable (for all other calls to request).

Let me know what you think about this solution. I am open to suggestions :)

@bidoubiwa
Copy link
Contributor

Hey @matthias-wright
Unfortunately, until this repo is not compatible with Meilisearch v0.28.0 your tests will not pass. I'll come back to you as soon as it is compatible!

Very sorry about the delay

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Aug 31, 2022

Hey @matthias-wright

The repo is now compatible with v0.28.0! There are a lot (a lot lot) of breaking changes in the code base.
If you'd like to adapt your contribution it would be very cool! If not, I completely understand as the code changed a lot.

Don't hesitate to close this PR if you prefer not!

@bidoubiwa
Copy link
Contributor

An alternative was introduced to be able to send document with jsonlines here: #417

@bidoubiwa bidoubiwa closed this May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants