build(deps): upgrade OpenDAL to 0.49.5 - #2015
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughOpenDAL was upgraded to 0.49.5 with version-aware native package copying and asynchronous ESM smoke tests. The harn registry now targets 0.10.35 binaries, while provider metadata updates a context limit and removes nine models. ChangesOpenDAL version alignment and validation
Registry and model metadata updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SmokeScript
participant OpenDALESM
participant NativePackages
SmokeScript->>OpenDALESM: dynamically import index.mjs
OpenDALESM-->>SmokeScript: return required constructors
SmokeScript->>OpenDALESM: perform memory write/read and construct S3 operator
SmokeScript->>NativePackages: resolve package for installed OpenDAL version
NativePackages-->>SmokeScript: return matching native package
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/afterPack.js (2)
246-256: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude actual identity in the error for easier debugging.
The thrown error for an invalid unpacked
opendalidentity omits the actualname/versionfound, which would speed up diagnosing packaging failures.💡 Proposed improvement
if (opendalPackageJson.name !== 'opendal' || typeof opendalPackageJson.version !== 'string') { - throw new Error(`Invalid unpacked opendal package identity at ${opendalDir}`) + throw new Error( + `Invalid unpacked opendal package identity at ${opendalDir}: name=${opendalPackageJson.name}, version=${opendalPackageJson.version}` + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/afterPack.js` around lines 246 - 256, Update the validation error in the opendal package identity check near opendalPackageJson to include the actual name and version values read from the unpacked package, while preserving the existing invalid-identity condition.
117-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicate version-aware package resolution logic across build/smoke scripts. Both scripts independently implement the same "walk node_modules/.pnpm-hoist/pnpm-virtual-store, optionally match name+version" algorithm; the shared logic risks diverging as one is CJS and the other ESM.
scripts/afterPack.js#L117-L148: extractresolveInstalledPackageDirinto a small shared helper module (e.g.scripts/lib/resolvePackage.js) usable from both CJS and ESM contexts.scripts/smoke-opendal-native.js#L85-L123: replacepackageMatches/resolvePackageDirFromNodeModuleswith the same shared helper instead of a parallel sync implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/afterPack.js` around lines 117 - 148, Extract the version-aware package resolution algorithm from resolveInstalledPackageDir in scripts/afterPack.js into a shared helper module usable by both CommonJS and ESM. Replace packageMatches and resolvePackageDirFromNodeModules in scripts/smoke-opendal-native.js with that helper, preserving package-name/version validation and pnpm virtual-store support; update both scripts to import and use the shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/acp-registry/registry.json`:
- Around line 712-748: Update normalizeBinaryTarget and
AcpRegistryBinaryDistribution to preserve and expose each binary target’s sha256
digest, then validate downloaded archives against that declared checksum before
extraction or execution. Apply this consistently to all platform targets; if
checksum validation is intentionally not implemented, remove the unused sha256
fields from the raw registry entries instead.
---
Nitpick comments:
In `@scripts/afterPack.js`:
- Around line 246-256: Update the validation error in the opendal package
identity check near opendalPackageJson to include the actual name and version
values read from the unpacked package, while preserving the existing
invalid-identity condition.
- Around line 117-148: Extract the version-aware package resolution algorithm
from resolveInstalledPackageDir in scripts/afterPack.js into a shared helper
module usable by both CommonJS and ESM. Replace packageMatches and
resolvePackageDirFromNodeModules in scripts/smoke-opendal-native.js with that
helper, preserving package-name/version validation and pnpm virtual-store
support; update both scripts to import and use the shared implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58fd4eae-ad44-4585-baad-57451df8c736
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
package.jsonresources/acp-registry/registry.jsonresources/model-db/providers.jsonscripts/afterPack.jsscripts/smoke-opendal-native.jstest/main/build/electronBuilderConfig.test.tstest/main/scripts/afterPack.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
opendaland all platform-specific native packages from0.49.2to0.49.5.Compatibility
0.49.5Linux x64 requiresGLIBC_2.38.Summary by CodeRabbit
New Features
Bug Fixes
Tests