Fix compilation errors from merge conflict in resource versioning#127
Conversation
Restored missing ResourceMetadata and ResourceInfo structs and get_build_info/get_resource_info functions that were lost during merge. Fixed return types to properly handle mixed Python types. Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
Replace deprecated pyo3::PyObject with Py<pyo3::types::PyAny> to comply with current PyO3 API Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
Replace env! with option_env! for CARGO_PKG_RUST_VERSION to handle cases where rust-version field is not set in Cargo.toml, improving robustness Co-authored-by: fbkaragoz <59958216+fbkaragoz@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR restores the resource metadata and versioning bindings that were accidentally dropped in a previous merge, and aligns the crate with a minimum Rust version. It reintroduces the Rust structures and Python-facing functions needed to inspect build metadata and embedded resources, and updates configuration/dependencies accordingly.
Changes:
- Reintroduced
ResourceInfo/ResourceMetadatastructs and theget_build_infoandget_resource_infoPython bindings insrc/lib.rs, wired into the PyO3 module. - Ensured build metadata is robust by reading
CARGO_PKG_RUST_VERSIONviaoption_env!with a"1.70"fallback and by parsing embeddedresources/metadata.json. - Declared
rust-version = "1.70"inCargo.tomland updatedCargo.lockto includeserde/serde_jsonand their transitive dependencies.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lib.rs | Restores resource metadata structs and Python APIs, parses embedded RESOURCE_METADATA, and registers the new functions in the PyO3 module. |
| Cargo.toml | Sets the crate’s minimum supported Rust version to 1.70 to match the metadata reported by get_build_info. |
| Cargo.lock | Updates lockfile to include serde, serde_json, and related dependencies needed for the new metadata parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let metadata: ResourceMetadata = serde_json::from_str(RESOURCE_METADATA) | ||
| .expect("Failed to parse embedded resource metadata"); |
There was a problem hiding this comment.
Using expect on serde_json::from_str(RESOURCE_METADATA) inside a #[pyfunction] will turn JSON parse failures into a Rust panic, which surfaces to Python as a less-informative error and can impact reliability; it would be safer to map the parse error into a PyErr (e.g., returning a Python exception) rather than panicking.
| /// print(resources['stopwords_base']['item_count']) # 442 | ||
| /// ``` | ||
| #[pyfunction] | ||
| fn get_resource_info(py: Python) -> PyResult<HashMap<String, Py<pyo3::types::PyAny>>> { |
There was a problem hiding this comment.
get_build_info/get_resource_info introduce new observable behavior but there are no unit tests covering them, while this file already has tests for other core functions; consider adding tests that exercise their happy-path behavior (e.g., keys present, item_count type) and failure modes (e.g., malformed RESOURCE_METADATA).
Merge commit 0cbc51e lost
ResourceMetadata,ResourceInfo, and their associated functions (get_build_info,get_resource_info) but retained module registration references, causing compilation failures.Changes
Restored missing code from commit 3f66674:
ResourceMetadataandResourceInfostructs with serde derivesget_build_info()andget_resource_info()Python bindingsFixed type handling:
Py<PyAny>instead of deprecatedPyObjectitem_countas int instead of string for Python interopAdded robustness:
option_env!()with fallback forCARGO_PKG_RUST_VERSIONrust-version = "1.70"to Cargo.tomlExample Usage
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.