add general upload method by McEazy2700 · Pull Request #18 · Lurk/cloudinary_rs

McEazy2700 · 2023-11-04T19:10:06Z

So I added the upload method to upload other kind of files like video and audio.

Sorry I didn't follow up on my last pull request, I was in school.

Lurk

Thank you for your contribution. Please check out the comments.

Lurk · 2023-11-06T21:01:22Z

src/lib.rs

+    ///         "cloud_name".to_string(),
+    ///         "api_secret".to_string()
+    ///     );
+    /// let options = UploadOptions::new().set_public_id("app_data.sql".to_string());


Let's use sane examples.

I actually did plan on saving some sql files to the cloud 🙂

Yeah, but you're passing it to a cloudinary.upload_image

Oh sorry about that. I've fixed the examples.

Lurk · 2023-11-06T21:13:50Z

src/upload/result.rs

-    pub format: String,
+    pub width: Option<usize>,
+    pub height: Option<usize>,
+    pub format: Option<String>,


Do you know if it is documented somewhere?

Yeah files like audio files, text files etc don't have width and height. There's a an example here

Instead of optional width params, I would like a more strict result type. If we upload an image, we know the result should have width, height, and format. And if we upload audio, it does not.

Lurk · 2023-11-06T21:32:48Z

src/lib.rs

+            "https://api.cloudinary.com/v1_1/{}/{}/upload",
+            self.cloud_name,
+            options
+                .get_resource_type()


I don't think upload options can be 100% shared between images, videos, and sound. For example, "faces" are relevant for images only.

From my understanding of the docs, the upload parameters are somewhat general. Perhaps cloudinary's api will accept the valid options based on the resource_type or the specific endpoint called, I'm nor really sure

Lurk · 2023-11-06T21:43:43Z

src/lib.rs

+        options: &UploadOptions<'_>,
+    ) -> Result<UploadResult> {
+        let new_opts = options.clone();
+        let updated_options = new_opts.set_resource_type(ResourceTypes::Image);


If options already contain resource type, we will silently overwrite it, which is a potential bug in a client application. We can catch it on compile time.

Yeah, I thought since the person is using the upload_image method, then the resource_type will have to be an image whether they set it or not

We can let them know that the overwrite will happen in the docs

How I image this API:

cloudinary.upload(Options::Image(...image_specific_options...))->UploadResultEnum; cloudinary.upload_image(...image_specific_options...)->...image_specific_result cloudinary.upload_audio(...audio_specific_options...)->...audio_specific_result

Yeah, I get this. So I've used the official Python SDK, and it doesn't have an upload_audio method, it just has an upload_image and an upload method that can still be used to upload images.

We'll probably need to determine the return type based on the values that are passed in as options. Hmmm.

I'm currently participating in a Hackathon that's ending Wednesday. Please I'll tackle this after.

Lurk · 2023-11-06T21:46:34Z

src/tests/mod.rs

+    let env_vars = load_env();
+    let cloudinary = Cloudinary::new(env_vars.api_key, env_vars.cloud_name, env_vars.api_secret);
+
+    let audio_url = "https://commons.wikimedia.org/wiki/File:Panama_National_Anthem.ogg";


I would like to have a small file locally.

Yeah that's fine, I just thought that for the test to run smoothly for everyone that clones the project, a url would work better.

Unfortunately, URLs tend to change at the most inconvenient moment. We have https://github.com/Lurk/cloudinary_rs/tree/main/assets in the repo.

Lurk · 2023-11-12T17:24:25Z

src/lib.rs

+    ///         "cloud_name".to_string(),
+    ///         "api_secret".to_string()
+    ///     );
+    /// let options = UploadOptions::new().set_public_id("app_data.sql".to_string());


Yeah, but you're passing it to a cloudinary.upload_image

Lurk · 2023-11-12T17:28:02Z

src/upload/result.rs

-    pub format: String,
+    pub width: Option<usize>,
+    pub height: Option<usize>,
+    pub format: Option<String>,


Instead of optional width params, I would like a more strict result type. If we upload an image, we know the result should have width, height, and format. And if we upload audio, it does not.

Lurk · 2023-11-12T17:35:43Z

src/lib.rs

+        options: &UploadOptions<'_>,
+    ) -> Result<UploadResult> {
+        let new_opts = options.clone();
+        let updated_options = new_opts.set_resource_type(ResourceTypes::Image);


How I image this API:

cloudinary.upload(Options::Image(...image_specific_options...))->UploadResultEnum; cloudinary.upload_image(...image_specific_options...)->...image_specific_result cloudinary.upload_audio(...audio_specific_options...)->...audio_specific_result

Lurk · 2023-11-12T17:38:37Z

src/tests/mod.rs

+    let env_vars = load_env();
+    let cloudinary = Cloudinary::new(env_vars.api_key, env_vars.cloud_name, env_vars.api_secret);
+
+    let audio_url = "https://commons.wikimedia.org/wiki/File:Panama_National_Anthem.ogg";


Unfortunately, URLs tend to change at the most inconvenient moment. We have https://github.com/Lurk/cloudinary_rs/tree/main/assets in the repo.

McEazy2700 added 2 commits November 4, 2023 20:07

add general upload method

2b8e69c

add documentation for upload() method

ac38a32

Lurk requested changes Nov 6, 2023

View reviewed changes

Lurk reviewed Nov 6, 2023

View reviewed changes

Lurk requested changes Nov 12, 2023

View reviewed changes

updated upload documentation

c96e97b

Lurk mentioned this pull request Dec 26, 2024

General upload method. #43

Open

Comments

Conversation

McEazy2700 commented Nov 4, 2023

Uh oh!

Lurk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lurk Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Lurk Nov 6, 2023 •

edited

Loading