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

Update examples.yml #19

Merged
merged 1 commit into from
Oct 23, 2023
Merged

Update examples.yml #19

merged 1 commit into from
Oct 23, 2023

Conversation

L-jasmine
Copy link
Contributor

No description provided.

Signed-off-by: zzz <458761603@qq.com>
Copy link
Member

juntao commented Oct 23, 2023

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


.github/workflows/examples.yml

The provided source code appears to be a YAML file for defining a GitHub Actions workflow named "examples". It consists of a set of jobs and steps to build and run different examples using WasmEdge.

Upon reviewing the code, here are some potential problems and recommendations:

  1. Security concerns in package installation:

    • The script uses sudo without verifying the source or command being executed. It is always recommended to exercise caution when using sudo. Make sure that the script is executed from a trusted source and that the commands being run with elevated privileges are safe.
    • Although it accepts the EULA (End User License Agreement) with ACCEPT_EULA=Y, it's always best to inform users about the agreement before accepting it.
  2. Installation of Rust target and WasmEdge:

    • The installation script assumes that rustup and curl utilities are already available and does not install them explicitly. Ensure that these dependencies are already installed or consider adding necessary steps to install them.
    • The versions of WasmEdge and its components (such as libtensorflowlite_c.so and libwasmedge_rustls.so) are hard-coded. It may be beneficial to parameterize these versions or check for the latest versions to ensure the most up-to-date installation.
  3. File paths and paths within the steps:

    • Make sure that the working directory (cd) is set correctly before executing commands. For example, in the "Client HTTPS example" step, navigate to the appropriate directory before running the build and execution commands.
    • Check if the filenames and file paths being used (wasmedge_hyper_client_https.wasm, grace_hopper.jpg, etc.) are correct and available in the specified locations.
  4. Redundant or missing commands:

    • In a few places, like the "Client example" step, the wasmedgec command is missing, which converts the Rust WASM binary to the WasmEdge binary format. Ensure that all necessary commands are present and in the correct order.
  5. Error handling and exit codes:

    • When running the examples, there are checks for success or failure using if statements. However, it might be more informative to use appropriate exit codes or log error messages to aid in troubleshooting.
  6. Missing documentation:

    • Consider adding comments or providing an overall description of the workflow and its purpose at the beginning of the file. This can help others understand the code and its intentions more easily.

Please note that this review is based on the provided code and problems might exist outside the context of the provided snippet.

The key changes in the provided patch are as follows:

  1. The command for downloading the wasmedge_rustls_plugin has been updated to download it from the official WasmEdge repository instead of the second-state repository. The new command is: wget https://github.com/WasmEdge/WasmEdge/releases/download/0.13.4/WasmEdge-plugin-wasmedge_rustls-0.13.4-ubuntu20.04_x86_64.tar.gz

  2. The command to extract the downloaded wasmedge_rustls_plugin has been updated to use tar instead of unzip. The new command is: tar -zxf WasmEdge-plugin-wasmedge_rustls-0.13.4-ubuntu20.04_x86_64.tar.gz

  3. The command to move the libwasmedge_rustls.so file to the /usr/local/lib/wasmedge directory has been updated. The new command is: sudo mv libwasmedge_rustls.so /usr/local/lib/wasmedge

These changes ensure that the latest version of the wasmedge_rustls_plugin is being downloaded from the official WasmEdge repository and correctly extracted and moved to the appropriate directory.

@juntao juntao merged commit 0582dff into WasmEdge:main Oct 23, 2023
2 checks passed
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