search

WasmEdge / docs

https://wasmedge.org/docs/
Apache License 2.0
17 stars 57 forks source link

Update client.md for HTTPS in reqwest #202

Closed juntao closed 9 months ago

juntao commented 10 months ago

Explanation

Add HTTPS section for reqwest and mysql_async

What type of PR is this

/kind documentation

alabulei1 commented 10 months ago

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.

Overall Summary:

The pull request titled "Update client.md for HTTPS in reqwest" contains several key changes related to enabling HTTPS support in reqwest. However, there are several potential issues and errors that need to be addressed.

The first problem is that the adaptation of reqwest_wasi mentioned in the patch does not yet support HTTPS. This could cause confusion for users who are expecting full HTTPS support.

The patch also lacks sufficient information about the TLS plugin and the wasmedge_rustls package. Providing more details about these dependencies would give users a better understanding of the requirements for HTTPS support.

Additionally, the patch removes information about the hyper API and the http_req API without any explanation. It would be beneficial to provide a brief reasoning for their removal.

There is a broken hyperlink to the wasmedge_reqwest_demo example, which needs to be fixed to ensure easy access for users.

Furthermore, the patch includes instructions on running examples without explaining their purpose or what they demonstrate. It's important to include this information to guide users better.

In the second set of summaries, there is confusion about the changes being made. The patch seems to update my_sql_driver.md instead of client.md. Clarification is needed in the patch description and subject.

The instructions for enabling TLS and running the application with TLS may not be complete and accurate. More details and explanations are necessary.

The instructions for running the application with TLS are specific to an AWS RDS database, which may not be applicable for other setups. Alternative information or options should be provided.

Finally, there is no mention of tests or validation for the changes made in the patch. It's unclear if the updates have been adequately tested.

Details

Commit 9e14d8c7f92887e0bd1a593a76283647aa79c023

Key Changes:

  1. The patch updates client.md in the docs/develop/rust/http_service/ directory to provide information about using HTTPS in the reqwest API.

  2. The patch adds detailed instructions on how to build and run reqwest examples with HTTPS support using the WasmEdge adapted reqwest_wasi crate.

  3. The patch adds information on how to install WasmEdge with the TLS plugin and enable the TLS feature in Cargo.toml to make HTTPS requests from reqwest.

Potential Problems:

  1. The patch mentions that the current adaptation of reqwest_wasi does not support HTTPS yet. This could confuse users who are expecting full HTTPS support.

  2. It would be helpful to include more information about the TLS plugin and the wasmedge_rustls package. This would provide users with a better understanding of the dependencies required for HTTPS support.

  3. The patch removes information about the hyper API and the http_req API, which were previously included in the client.md file. It would be beneficial to provide a brief explanation or reasoning for their removal.

  4. The patch includes a broken hyperlink to the wasmedge_reqwest_demo example in the reqwest section. This should be fixed to ensure users can access the example code easily.

  5. The patch provides instructions on how to run the examples, but it does not include an explanation of what the examples do or what they demonstrate. This information should be included to guide users better.

Commit e1e8efd223ecdbe6b3f09a9e042900d073183dd9

Key changes:

Potential problems: