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.

Sign up for GitHub

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

Jump to bottom

[Installer] Support WasmEdge rustls plugin installation #3032

Merged
merged 1 commit into from
Nov 13, 2023
Merged

[Installer] Support WasmEdge rustls plugin installation #3032

merged 1 commit into from
Nov 13, 2023

Conversation

hydai
Copy link
Member

@hydai hydai commented Nov 13, 2023

No description provided.

@hydai hydai requested a review from SAtacker November 13, 2023 02:39
@juntao Flows.network Integration
Copy link
Member

juntao commented Nov 13, 2023 •
edited
Loading

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

Commit 7782a84c9b386074920da138364f3512e786be27

This Pull Request introduces changes that add support for the WasmEdge rustls plugin installation. Here is a summary of the key changes:

  1. There are modifications to the test workflow file (test-python-install-script.yml), adding steps to test the installation of the wasmedge_rustls plugin. Specifically, it adds checks for the installation of the .so and .dylib shared libraries of the Rustls plugin.

  2. The install.py Python script, which is a part of the installation utility, has been updated to recognize the WasmEdge rustls plugin. The new plugin is added to the PLUGINS_AVAILABLE list and the respective version details are appended to the MAP_NAME_LIST.

As for potential problems to consider:

  1. This code assumes that wasmedge_rustls plugin version 0.13.4 works adequately with the existing versions of other components and there wouldn't be any compatibility issues.

  2. The tests for the plugin are not run on systems identified as 'manylinux2014 aarch64'. It needs to be ensured that this is an intentional exclusion and isn't necessary for the environments the software will be deployed to.

  3. The Pull Request does not contain any provisions to handle errors or exceptions that might occur during the installation of the Rustls plugin. It might be prone to fail without significant error information if anything goes wrong during the installation.

  4. The changes currently lack documentation updates. Adding notes about this new plugin, its version compatibility, and the updated installation script would help keep everything clear for both developers and users.

@github-actions github-actions bot added the c-Installer An issue related to WasmEdge installer label Nov 13, 2023
@hydai hydai force-pushed the hydai/enable_rustls_in_installer branch from 8d53953 to 82dcde4 Compare November 13, 2023 02:42
SAtacker
SAtacker reviewed Nov 13, 2023
View reviewed changes
utils/install.py
@@ -387,6 +389,10 @@ def compare(self, version2, separator=". |-", ignorecase=True):
"manylinux2014" + "x86_64" + WASMEDGE_IMAGE_PLUGIN: VersionString("0.13.0"),
"manylinux2014" + "aarch64" + WASMEDGE_IMAGE_PLUGIN: VersionString("0.13.0"),
"ubuntu20.04" + "x86_64" + WASMEDGE_IMAGE_PLUGIN: VersionString("0.13.0"),
"darwin" + "x86_64" + WASMEDGE_RUSTLS: VersionString("0.13.4"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a release for this, is there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here: https://github.com/WasmEdge/WasmEdge/releases/tag/0.13.4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, for 0.13.5 it isn't there

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/WasmEdge/WasmEdge/releases/download/0.13.5/WasmEdge-plugin-wasmedge_rustls-0.13.5-darwin_x86_64.tar.gz

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah... The reason is that the macOS-11 CI runner is broken. So, all artifacts are not built and updated. We are still fixing this issue. If it's working again, then the release assets will be updated soon.

utils/install.py Show resolved Hide resolved
@hydai
[Installer] Support WasmEdge rustls plugin installation
7782a84 
Signed-off-by: hydai <z54981220@gmail.com>
@hydai hydai force-pushed the hydai/enable_rustls_in_installer branch from 82dcde4 to 7782a84 Compare November 13, 2023 02:57
@github-actions github-actions bot added the c-CI An issue related to CI label Nov 13, 2023
@hydai
Copy link
Member Author

hydai commented Nov 13, 2023 •
edited
Loading

The macOS-lastest will fail as expected Because the related 0.13.5 assets are not built. See #3013

@SAtacker
Copy link
Collaborator

Just noting here the url so we don't have to read the whole log again https://github.com/WasmEdge/WasmEdge/releases/download/0.13.5/WasmEdge-plugin-wasmedge_tensorflow-0.13.5-darwin_x86_64.tar.gz

@juntao
Copy link
Member

juntao commented Nov 13, 2023

Can we merge?

@SAtacker SAtacker merged commit 46c9257 into master Nov 13, 2023
21 of 22 checks passed
@SAtacker SAtacker deleted the hydai/enable_rustls_in_installer branch November 13, 2023 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SAtacker SAtacker SAtacker approved these changes

@ibmibmibm ibmibmibm Awaiting requested review from ibmibmibm

@q82419 q82419 Awaiting requested review from q82419

Assignees
No one assigned
Labels
c-CI An issue related to CI c-Installer An issue related to WasmEdge installer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hydai @juntao @SAtacker