Skip to content
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

add support for Csv to rama::http similar to Json support we have #394

Closed
GlenDC opened this issue Jan 14, 2025 · 5 comments
Closed

add support for Csv to rama::http similar to Json support we have #394

GlenDC opened this issue Jan 14, 2025 · 5 comments
Assignees
Labels
easy An easy issue to pick up for anyone. enhancement New feature or request good first issue Good for newcomers mentor available A mentor is available to help you through the issue.
Milestone

Comments

@GlenDC
Copy link
Member

GlenDC commented Jan 14, 2025

For the serde-support you can make use of https://docs.rs/csv/latest/csv.

Note, regarding making a response, for the time being ContentType does not have a csv constructor, so instead you'll use a non-typed approach, e.g.:

[(
    CONTENT_TYPE,
    HeaderValue::from_str(mime::TEXT_CSV).unwrap(),
)],
buf.into_inner().freeze(),
@GlenDC GlenDC added enhancement New feature or request good first issue Good for newcomers easy An easy issue to pick up for anyone. mentor available A mentor is available to help you through the issue. labels Jan 14, 2025
@GlenDC GlenDC added this to the v0.2 milestone Jan 14, 2025
@IamTossan
Copy link
Contributor

In https://docs.rs/rama-http-types/0.2.0-alpha.6/src/rama_http_types/response/json.rs.html#26,
we have an example and a bit below, we got: pub struct Json<T>(pub T);
What could be expected for the inner type for the Csv version?

@GlenDC
Copy link
Member Author

GlenDC commented Jan 17, 2025

Do you wish to pick up this issue @IamTossan , if so I'll gladly assign it to you.

You would add these trait bounds for the locations where you actually serialize to csv, which I think is only IntoResponse impl:

impl<T> IntoResponse for Csv<T>
where
    T: IntoIterator<Item: Serialize>,
{
   // ...
}

As that are also the trait bounds enforced for the serde-supported API of the csv crate.

Best to probably put sufficient documentation (rustdocs) for the Csv type declaration, and also link to external resources such as https://docs.rs/csv/latest/csv/struct.Writer.html#rules as these also explain what is possible and what not. Csv is not as flexible as json so certain data compositions will result in Serialize::Error's. That's okay, and not much to do about that AFAIK, but at the very least we can properly document it, also add a test and doc example.

@IamTossan
Copy link
Contributor

Hi @GlenDC ,
thanks for your answer.
I'd like to be assigned and I'll try my best, thank you.

@GlenDC
Copy link
Member Author

GlenDC commented Jan 22, 2025

Amazing @IamTossan . Do let me know if you need help or are stuck.
Feel free to open a PR whenever you feel ready, have questions or need guidance.

I'll assign it to you.

@GlenDC
Copy link
Member Author

GlenDC commented Jan 29, 2025

Closed in #401 by @IamTossan . Thanks for another great contribution @IamTossan ! Take care.

@GlenDC GlenDC closed this as completed Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy An easy issue to pick up for anyone. enhancement New feature or request good first issue Good for newcomers mentor available A mentor is available to help you through the issue.
Projects
None yet
Development

No branches or pull requests

2 participants