Add SAMM cli usage #33
Conversation
| import sys | ||
| import zipfile | ||
|
|
||
| BASE_PATH = "https://github.com/eclipse-esmf/esmf-sdk/releases/download/v2.6.1/" |
There was a problem hiding this comment.
Perhaps it's worth extracting the version of CLI to config, or at least to a constant?
There was a problem hiding this comment.
use a constant for the SAMM CLI version
|
|
||
|
|
||
| class SammCli: | ||
| """Class to eecute SAMM CLI functions. |
There was a problem hiding this comment.
| """Class to eecute SAMM CLI functions. | |
| """Class to execute SAMM CLI functions. |
There was a problem hiding this comment.
Fixed
| class SammCli: | ||
| """Class to eecute SAMM CLI functions. | ||
|
|
||
| If there is no downloaded SAMM CLI, code will identify operation system and download a corresponding client. |
There was a problem hiding this comment.
| If there is no downloaded SAMM CLI, code will identify operation system and download a corresponding client. | |
| If there is no downloaded SAMM CLI, the code will identify the operating system and download a corresponding SAMM CLI version. |
There was a problem hiding this comment.
Fixed
| If there is no SAMM Cli executable file, run a script for downloading. | ||
| """ | ||
| if not exists(self._samm): | ||
| # download SAMM CLI |
There was a problem hiding this comment.
Remove this comment, this is redundant
There was a problem hiding this comment.
Removed
| return cli_path | ||
|
|
||
| def _validate_client(self): | ||
| """Validate SAMM Cli. |
There was a problem hiding this comment.
Please unify the name, e.g. use "SAMM CLI" everywhere
There was a problem hiding this comment.
Changed for all comments
| param path_to_model: local path to the aspect model file (*.ttl) | ||
| possible arguments: | ||
| - output, o: output file path (default: stdout) | ||
| - api-base-url, b: the base url for the Aspect API used in the OpenAPI specification, b="http://mysite.de" |
There was a problem hiding this comment.
Please something neutral as example, e.g. http://example.com/ or http://localhost/. http://mysite.de is an actually existing website.
There was a problem hiding this comment.
Updated.
Note: This description is taken from the ESMF documentation (https://eclipse-esmf.github.io/esmf-developer-guide/tooling-guide/samm-cli.html)
| import sys | ||
| import zipfile | ||
|
|
||
| BASE_PATH = "https://github.com/eclipse-esmf/esmf-sdk/releases/download/v2.6.1/" |
There was a problem hiding this comment.
IMO it would make sense to define the version string ("2.6.1") as a variable or constant, since it's repeated many times over.
There was a problem hiding this comment.
Use a separate constant for the version number
| print("Start extracting files") | ||
| archive = zipfile.ZipFile(archive_file) | ||
| for file in archive.namelist(): | ||
| archive.extract(file, ".\\samm-cli") |
There was a problem hiding this comment.
Does this work on all platforms (because of the backslash \)?
There was a problem hiding this comment.
Replaces with os specific separator
| print(f"Start downloading SAMM CLI {samm_cli_file_name}") | ||
| url = BASE_PATH + samm_cli_file_name | ||
| dir_path = Path(__file__).resolve().parent | ||
| archive_file = os.path.join(dir_path, f".\\{samm_cli_file_name}") |
There was a problem hiding this comment.
I think you have to use:
archive_file = os.path.join(dir_path, samm_cli_file_name)
so that it works for all os systems.
There was a problem hiding this comment.
Updated
| print("Start extracting files") | ||
| archive = zipfile.ZipFile(archive_file) | ||
| for file in archive.namelist(): | ||
| archive.extract(file, ".\\samm-cli") |
There was a problem hiding this comment.
I think you have to use, also here something like:
archive.extract(file, os.path.join("", "samm-cli"))
There was a problem hiding this comment.
Updated
Changes: