feat: expose bundled AWS SDK version - #1715
Conversation
richarddavison
left a comment
richarddavison
left a comment
There was a problem hiding this comment.
We could instead set here:
llrt/modules/llrt_process/src/lib.rs
Line 142 in ff69064
richarddavison
left a comment
richarddavison
left a comment
There was a problem hiding this comment.
Thanks! Just a minor suggestion and we're good to go!
| fn create_process_versions<'js>( | ||
| ctx: &Ctx<'js>, | ||
| aws_sdk_version: Option<&str>, | ||
| ) -> Result<Object<'js>> { | ||
| let versions = Object::new(ctx.clone())?; | ||
| versions.set("llrt", VERSION)?; | ||
| // Node.js version - Set for compatibility with some Node.js packages (e.g. cls-hooked). | ||
| versions.set("node", "0.0.0")?; | ||
|
|
||
| if let Some(version) = aws_sdk_version { | ||
| versions.set("@aws-sdk", version)?; | ||
| } | ||
|
|
||
| Ok(versions) | ||
| } | ||
|
|
There was a problem hiding this comment.
Instead of passing option_env!("LLRT_AWS_SDK_VERSION") we should just use it in here directly:
if let Some(version) = option_env!("LLRT_AWS_SDK_VERSION") {
|
|
||
| assert_eq!(versions.get::<_, String>("llrt").unwrap(), VERSION); | ||
| assert_eq!(versions.get::<_, String>("node").unwrap(), "0.0.0"); | ||
| assert_eq!(versions.get::<_, String>("@aws-sdk").unwrap(), "3.1057.0"); |
There was a problem hiding this comment.
Can the SDK version be specified dynamically? Otherwise, I think the tests will fail during future SDK version upgrades.
There was a problem hiding this comment.
The version is actually injected to make the code testable. I think we can skip this test entirely and just check contains a value.
3 participants
Summary
process.versions["@aws-sdk"]@aws-sdk/client-stspackage during SDK bundlingllrt_processconstructsprocess.versionsNONEbundles and Cargono-sdkbuildsWhy
LLRT does not bundle AWS SDK package metadata, so applications cannot reliably import a client package's
package.jsonto determine the bundled SDK version.process.versionsalready reports LLRT and Node compatibility versions and provides a runtime-visible location for this metadata.Closes #1446.
Implementation
The JavaScript build writes
bundle/js/.aws-sdk-versiononly after a standard or full SDK bundle succeeds. A build script owned byllrt_processreads that marker and embeds it asLLRT_AWS_SDK_VERSIONwhen compiling the process module.llrt_process::initthen adds the value while constructingprocess.versions.The existing
no-sdkfeature is forwarded throughllrt_coreandllrt_modulestollrt_process. This makes Cargono-sdkauthoritative even if a marker from an earlier SDK bundle remains present.Review Feedback
llrt_coreembedded-runtime initializationprocess.versionsconstruction, and tests inmodules/llrt_process, as requestedLLRT_AWS_SDK_VERSIONdirectly while constructingprocess.versionsValidation
cargo check --workspacecargo test -p llrt_process— 3 passed, 0 failedcargo test -p llrt_process --features no-sdk— 3 passed, 0 failedcargo clippy -p llrt_process --tests -- -D warningsgit diff --checkRuntime proof:
SDK_BUNDLE_MODE=STD3.1057.0{"hasAwsSdk":true,"version":"3.1057.0"}SDK_BUNDLE_MODE=NONE{"hasAwsSdk":false,"version":null}--features no-sdk3.1057.0{"hasAwsSdk":false,"version":null}The workspace was restored to a normal STD build after the matrix, which reports
process.versions["@aws-sdk"] === "3.1057.0".