feat: add tls backend control through features to rust api client#443
feat: add tls backend control through features to rust api client#443Alenar wants to merge 1 commit intoblockfrost:masterfrom
Conversation
by upgrading the generator from `0.7.12` to `0.7.20` This also upgrade `reqwest` to `0.13+`.
|
@Alenar is attempting to deploy a commit to the Five Binaries IOG Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Updates the generated Rust API client to allow selecting the reqwest TLS backend via crate features, enabled by upgrading the OpenAPI generator and regenerating the Rust crate.
Changes:
- Upgrade OpenAPI generator configuration (7.12.0 → 7.20.0) and regenerate Rust client output.
- Update Rust crate to
reqwest ^0.13withdefault-features = falseand addnative-tls/rustlscrate features to control TLS backend. - Regenerated model changes include some public enum variant renames (e.g.,
Array→ArrayVecString).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/src/models/signer_registrations_list_item_message.rs | Regenerated model field ordering / doc adjustments. |
| rust/src/models/register_signer_message.rs | Regenerated model field ordering / doc adjustments. |
| rust/src/models/asset_onchain_metadata_cip25_image.rs | Regenerated enum variant rename affecting public API. |
| rust/src/models/asset_onchain_metadata_cip25_files_inner_src.rs | Regenerated enum variant rename affecting public API. |
| rust/src/models/asset_onchain_metadata_cip25_description.rs | Regenerated enum variant rename affecting public API. |
| rust/README.md | Bumps recorded generator version in generated README. |
| rust/Cargo.toml | Switch to reqwest ^0.13 and add TLS-backend selection features. |
| openapitools.json | Bumps generator-cli version to 7.20.0. |
Comments suppressed due to low confidence (1)
rust/README.md:224
- This introduces new crate features (
native-tls/rustls) to control the TLS backend, but the README doesn’t document how consumers should select them (including the need to usedefault-features = falsewhen opting intorustlsto avoid also pullingnative-tlsvia the default feature). Please add a short note in the Installation section showing the intended Cargo dependency snippets.
- Generator version: 7.20.0
- Build package: `org.openapitools.codegen.languages.RustClientCodegen`
## Installation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; | |||
| #[serde(untagged)] | |||
| pub enum AssetOnchainMetadataCip25FilesInnerSrc { | |||
| String(String), | |||
There was a problem hiding this comment.
Renaming this public enum variant (Array → ArrayVecString) is a breaking API change for downstream crates that pattern-match on the old variant. If this crate is published, please ensure the release/versioning reflects the breaking change (or provide a compatibility alias/deprecation path if you need to minimize churn).
| String(String), | |
| String(String), | |
| /// Deprecated alias for `ArrayVecString` to preserve backward compatibility. | |
| #[deprecated(note = "Variant `Array` has been renamed to `ArrayVecString`. Please update your code to use `AssetOnchainMetadataCip25FilesInnerSrc::ArrayVecString` instead.")] | |
| Array(Vec<String>), |
| @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; | |||
| #[serde(untagged)] | |||
| pub enum AssetOnchainMetadataCip25Description { | |||
| String(String), | |||
There was a problem hiding this comment.
Renaming this public enum variant (Array → ArrayVecString) is a breaking API change for downstream crates that pattern-match on the old variant. If this crate is published, please ensure the release/versioning reflects the breaking change (or provide a compatibility alias/deprecation path if you need to minimize churn).
| String(String), | |
| String(String), | |
| #[deprecated(note = "Renamed to `ArrayVecString`. This alias will be removed in a future major release.")] | |
| Array(Vec<String>), |
| [features] | ||
| default = ["native-tls"] | ||
| native-tls = ["reqwest/native-tls"] | ||
| rustls = ["reqwest/rustls"] |
There was a problem hiding this comment.
Cargo.lock in rust/ still pins reqwest to 0.12.x, but Cargo.toml now requires ^0.13. Please regenerate/update rust/Cargo.lock (e.g., run cargo update -p reqwest or cargo build) so CI and local builds use the intended reqwest version and feature set.
| rustls = ["reqwest/rustls"] | |
| rustls = ["reqwest/rustls-tls"] |
| "generator-cli": { | ||
| "version": "7.12.0" | ||
| "version": "7.20.0" | ||
| } |
There was a problem hiding this comment.
The PR description says the generator was upgraded from 0.7.12 to 0.7.20, but this config (and the Rust README) uses 7.12.0 → 7.20.0. Please align the version notation in the PR description or the config to avoid confusion about which generator major version is being used.
| @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; | |||
| #[serde(untagged)] | |||
| pub enum AssetOnchainMetadataCip25Image { | |||
| String(String), | |||
There was a problem hiding this comment.
Renaming this public enum variant (Array → ArrayVecString) is a breaking API change for downstream crates that pattern-match on the old variant. If this crate is published, please ensure the release/versioning reflects the breaking change (or provide a compatibility alias/deprecation path if you need to minimize churn).
| String(String), | |
| String(String), | |
| /// Deprecated: use `ArrayVecString` instead. | |
| #[deprecated(note = "renamed to ArrayVecString to be more explicit; this alias will be removed in a future major release")] | |
| Array(Vec<String>), |
Content
This PR add tls backend control through features to the rust api client.
This was done by upgrading the generator from
0.7.12to0.7.20and regenerating the rust crate usingyarn generate-types:rust.Rational
At Mithril one of our current focus is switching all our binaries to static linking, for TLS we want to switch our backend from
native-tls-vendoredtorustlsto remove all paths that build openssl but ... thisblockfrost-openapicrate doesn't allow to control the features of itsreqwestdependency as it is declared like this:Meaning that it activate the default feature of reqwest which spread to our crates through the
blockfrostcrate.Looking at the OpenApi generator project I found out that they added TLS backend control last year since version
7.13.0and it was futher enhanced with later versions (e.g with configurable default backend in7.17.0).Additional impact
The generator also upgraded
reqwestto0.13+, which have some impacts (e.g. default backend switching fromnative-tlstorustls,rustls-tlsfeature renamed torustls)Another auto-generated change is the renaming of some enums variants from
ArraytoArrayVecString, I took a look at theblockfrost-rustcrate and those variants were not used here, so it should be fine as long as third party crates are not using them (which I don't know).Next
To allow full control of the TLS backend by child crates the
blockfrost-rustCargo.toml have to be updated to propagate the already available backend choice to the itsblockfrost-openapidependency.A base patch could be this one:
Several notes:
blockfrost-openapi0.1.84version is tentative.reqwestis upgraded to0.13.2to be consistent with the version now used inblockfrost-openapinative-tlsfeature since before native-tls backend could only be set though thedefault-tlsfeature which have changed torustlsrustls-tlsfeature torustlslike what reqwest didI can do a PR on the
blockfrost-rustrepository but I feel that those choices are not neutral, so they should be made by core maintainers.