Boavizta / boaviztapi

🛠 Giving access to BOAVIZTA reference data and methodologies trough a RESTful API
GNU Affero General Public License v3.0
66 stars 23 forks source link

Generate SDK based on openAPI #84

Closed da-ekchajzer closed 1 year ago

da-ekchajzer commented 2 years ago

Problem

We should provide SDK at each release to help people implement project using the API.

Solution

Use library that's generate SDK based on the OpenAI specification during the gitlfow release. At first, we should provide the SDK for the tow main languages used by the community :

We could use : https://github.com/OpenAPITools/openapi-generator which can generate SDK for most of the available languages.

demeringo commented 2 years ago

Hi David and others,

As a temporary solution for the needs of the cloud-scanner, I generated (and committed) a Rust SDK inside the cloud-scanner repo. Can be found here: https://github.com/Boavizta/cloud-scanner/tree/main/boavizta-api-sdk . I did not pusblish the sdk as a public crate yet, but yes it would make sense to publish it for several languages with each new release of the API.

I used the following command (note the flag to set the name of the crate/package as boavizta_api_sdk instead of default "openapi something").

docker run --rm -v "${PWD}:/local" openapitools/openapi-generator-cli generate -i http://api.boavizta.org/openapi.json   -g rust  -o /local/boavizta-api-sdk --package-name boavizta_api_sdk

All went well

bpetit commented 2 years ago

This is done and working in boagent for python. Needs some documentation though.

demeringo commented 2 years ago

It could be nice to publish the rust crate for boavizta api sdk.

bpetit commented 2 years ago

I created an issue to remind myself about this : https://github.com/Boavizta/boagent/issues/17

But I'll receive all warnings about doing this in a distinct repository :)

(I think this is clearer this way, but I may be wrong)

bpetit commented 2 years ago

It could be nice to publish the rust crate for boavizta api sdk.

the same for a python pip package :)

da-ekchajzer commented 2 years ago

Do you agree that it's should be done on each release ?

bpetit commented 2 years ago

I do ! So I guess we should automate, is this what you were thinking about ?

da-ekchajzer commented 2 years ago

I do also. On each release. I am not the best at GitHub workflow, so I could propose the project during the hackathon or to other contributors.

bpetit commented 2 years ago

When you release a new version of the API, you push a tag first, right ?

I'm trying to set the version of the sdk depending on the API version and ensure it's pushed only when a release is created.

da-ekchajzer commented 2 years ago

Yes the tag and the version name are always the same : vx.y.z

da-ekchajzer commented 2 years ago

The version number can be found in /boaviztapi/init.py

da-ekchajzer commented 2 years ago

Can we close this issue and open another for the cargo (rust) package ?

da-ekchajzer commented 2 years ago

We got an issue with the workflow : https://github.com/Boavizta/boaviztapi/runs/6921383671?check_suite_focus=true

ERROR    HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/
         File already exists. See https://pypi.org/help/#file-name-reuse for    
         more information.                                                      
Error: Process completed with exit code 1.

It is due to the fact that the sdk name does not take the right version name boaviztapi-sdk-0.0.0.tar.gz instead of boaviztapi-sdk-0.1.2.tar.gz. Since the file already exist in pypi, the workflow crashes.

demeringo commented 2 years ago

I think @bpetit nailed the root cause in his latests debug commits.

Env vars are not persisted accross multiples steps (explaining the empty version in action... while it works on my machine ;-) )

Maybe we may need to use something like this (but I did not try)

steps:
  # This will set the FOO environment variable within the
  # first `run` script, then instruct GitHub Actions to make
  # it available within subsequent `run` steps as well.
  - run: |
      export FOO=bar
      echo "::set-env name=FOO::$FOO"
  - run: echo $FOO

Thanks https://www.edwardthomson.com/blog/github_actions_15_sharing_data_between_steps.html

demeringo commented 2 years ago

Latest version of openapitools/openapi-generator-cli generates Rust code that does not compile.

A temporary workaroud is to generate the code with older version v6.0.0 (openapitools/openapi-generator-cli:v6.0.0)

docker run --rm -v "${PWD}:/local" openapitools/openapi-generator-cli:v6.0.0 generate -i https://pwd6mhj7ej.execute-api.eu-west-1.amazonaws.com/dev/openapi.json -g rust -o /local/boavizta-api-sdk --package-name boavizta_api_sdk

Note:

A non blocking issue with variable naming remains (clippy complains that "structure field TYPE should have a snake case name".

It seems due to the fact that python API has a field named type. This is a keyword in Rust, so the generator renames it as TYPE which in turns makes clippy (the Rust linter) unhappy.

demeringo commented 2 years ago

@bpetit as expected the generated codes still creates clippy warnings:

https://github.com/Boavizta/boaviztapi/runs/7305774236?check_suite_focus=true#step:9:188

We could get rid of this by patching the generated code before publishing it ... or let it has it is. What do you think ?

We could patch with the following code:

echo "âš  Rename the field \`TYPE`` into \`usage_type\` to comply with Rust naming conventions"
echo "âš  This is really a hacky workaround that we should remove when the code generation is fixed" 
sed -i "s/pub TYPE: Option<String>,/pub usage_type: Option<String>,/" src/models/usage_cloud.rs
sed -i "s/TYPE: None,/usage_type: None,/" src/models/usage_cloud.rs
sed -i "s/pub TYPE: Option<String>,/pub usage_type: Option<String>,/" src/models/usage_server.rs
sed -i "s/TYPE: None,/usage_type: None,/" src/models/usage_server.rs
demeringo commented 2 years ago

Also need to add the 2 following lines to force the reqwest package to not use default featuresand use rustls-tls instead.

This allow to get rid of OpenSSL native dependencies that make code difficult to compile cross platform, like deploying as serverless.

# Configure reqwest to use rustls-tls instead of native deps to OpenSSL
sed -i "/^\[dependencies.reqwest\].*/a default-features = false" Cargo.toml
sed -i "s/^features.*/features = [\"json\", \"multipart\", \"rustls-tls\"]/" Cargo.toml
demeringo commented 2 years ago

Haha, Crates.io refuses that we publish a package when the code is not in publicly available git ;-)

This is a very sensible idea in itselft, but I had forgotten this requirement 😲! I guess we will have to manage the generated sdk as a repository.

Extract from the failing GH action ouptut. https://github.com/Boavizta/boaviztapi/runs/7307126443?check_suite_focus=true#step:10:24

Run katyo/publish-crates@v1
Searching cargo packages at './boaviztapi_sdk'
Found packages: boaviztapi_sdk
Checking packages consistency
Sorting packages according to dependencies
Publishing package 'boaviztapi_sdk'
/home/runner/.cargo/bin/cargo publish
    Updating crates.io index
warning: manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
error: 41 files in the working directory contain changes that were not yet committed into git:
Cargo.toml
README.md
docs/ValidationError.md
docs/ComponentApi.md
docs/UsageServer.md
docs/MotherBoard.md
docs/UsageCloud.md
docs/Disk.md
docs/Cpu.md
docs/Case.md
docs/ServerApi.md
docs/ConfigurationServer.md
docs/PowerSupply.md
docs/ServerDto.md
docs/LocationInner.md
docs/CloudApi.md
docs/Ram.md
docs/ModelServer.md
docs/HttpValidationError.md
src/apis/component_api.rs
src/apis/mod.rs
src/apis/server_api.rs
src/apis/configuration.rs
src/apis/cloud_api.rs
src/lib.rs
src/models/location_inner.rs
src/models/mother_board.rs
src/models/cpu.rs
src/models/model_server.rs
src/models/disk.rs
src/models/usage_cloud.rs
src/models/validation_error.rs
src/models/case.rs
src/models/mod.rs
src/models/http_validation_error.rs
src/models/configuration_server.rs
src/models/power_supply.rs
src/models/ram.rs
src/models/server_dto.rs
src/models/usage_server.rs
git_push.sh
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
Error: Error: The process '/home/runner/.cargo/bin/cargo' failed with exit code 101
demeringo commented 2 years ago

Metadata to provide before publishing to crate.io:

https://doc.rust-lang.org/cargo/reference/publishing.html#before-publishing-a-new-crate

demeringo commented 1 year ago

See also https://www.printlnhello.world/blog/publishing-to-crates-io/#then-do-it-again-on-staging. It describes how to test publication on crate.io staging environment first.

And https://codeandbitters.com/published-crate-analysis/ for best practices around publishing crate and link to the source code.

demeringo commented 1 year ago

Hello, closing the issue as we now have a proper Rust SDK in https://github.com/Boavizta/boaviztapi-sdk-rust