Rust bindings v1#284
Draft
Choochmeque wants to merge 79 commits into
Draft
Conversation
…xample, because it should be reusable-action)
…is, and move_data
… fdb-sys and fdb crates
…r config handling
…r archiving functionality
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #284 +/- ##
===========================================
- Coverage 71.15% 71.02% -0.13%
===========================================
Files 370 370
Lines 23455 23455
Branches 2463 2463
===========================================
- Hits 16690 16660 -30
- Misses 6765 6795 +30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust FDB bindings to use the higher-level eckit/metkit Rust crates directly (instead of a custom Request/DataReader layer), and adapts downstream tools/tests/examples to the new API shape.
Changes:
- Replace
fdb::Requestwithmetkit::MarsRequestand migrate call sites accordingly. - Replace the custom
DataReaderwitheckit::DataHandle+open_for_read()for streaming reads. - Introduce
UserConfigand new C++ bridge plumbing (includingMessageArchiver) and updatefdb-hammerto match.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/Cargo.toml | Switch workspace deps to local-path eckit/metkit/bindman sources (currently non-reproducible). |
| rust/tools/fdb-hammer/src/main.rs | Migrate hammer to metkit::CodesHandle, metkit::MarsRequest, and new Fdb::open signature. |
| rust/tools/fdb-hammer/README.md | Update runtime instructions for hammer (RPATH/env-var guidance text). |
| rust/tools/fdb-hammer/Cargo.toml | Add eckit/metkit deps; add build-dep for rpath helper. |
| rust/tools/fdb-hammer/build.rs | Emit RPATH entries for runtime shared-library discovery. |
| rust/crates/fdb/src/lib.rs | Public API re-exports updated: remove Request/DataReader, add UserConfig/MessageArchiver. |
| rust/crates/fdb/src/handle.rs | Core API updated to accept metkit::MarsRequest and return eckit::DataHandle; add MessageArchiver. |
| rust/crates/fdb/src/options.rs | Add UserConfig overlay config type and conversion into eckit::Config. |
| rust/crates/fdb/src/error.rs | Simplify error mapping to wrap eckit::Error directly. |
| rust/crates/fdb/src/request.rs | Remove custom Request wrapper implementation. |
| rust/crates/fdb/src/datareader.rs | Remove custom DataReader wrapper implementation. |
| rust/crates/fdb/README.md | Document downstream binary RPATH handling via bindman_utils::emit_rpaths(). |
| rust/crates/fdb/build.rs | Change build-script behavior to re-export dependency root metadata. |
| rust/crates/fdb/Cargo.toml | Update crate metadata/deps to include eckit/metkit, adjust links. |
| rust/crates/fdb/examples/fdb_retrieve.rs | Update example to parse requests via metkit and read via DataHandle. |
| rust/crates/fdb/examples/fdb_read.rs | Update example to parse requests via metkit and stream via DataHandle. |
| rust/crates/fdb/examples/fdb_list.rs | Update example to parse requests via metkit and call Fdb::list with MarsRequest. |
| rust/crates/fdb/examples/fdb_axes.rs | Update example to parse requests via metkit and call Fdb::axes with MarsRequest. |
| rust/crates/fdb/examples/fdb_archive.rs | Update example to open config via eckit::Config::from_path. |
| rust/crates/fdb/benches/fdb_bench.rs | Update benchmarks to build MarsRequest and initialise eckit where needed. |
| rust/crates/fdb/tests/fdb_integration.rs | Update integration tests to use eckit::Config, MarsRequest, and DataHandle IO. |
| rust/crates/fdb/tests/fdb_async.rs | Update async tests to build MarsRequest from Key and read via DataHandle. |
| rust/crates/fdb/tests/fdb_thread_safety.rs | Update thread-safety tests to new request type and config handling (currently non-hermetic). |
| rust/crates/fdb-sys/src/lib.rs | Rework CXX interface: take MarsRequestWrapper and ConfigWrapper; return DataHandleWrapper. |
| rust/crates/fdb-sys/cpp/fdb_bridge.h | Update bridge header to include metkit/eckit wrapper headers; add MessageArchiver wrappers. |
| rust/crates/fdb-sys/cpp/fdb_bridge.cpp | Implement new bridge functions for list/retrieve/axes/etc using MarsRequestWrapper. |
| rust/crates/fdb-sys/build.rs | Generate exception bridge; update include/link metadata and vendored/system build logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+24
| eckit-sys = { path = "../../rust-wrappers-playground/crates/eckit-sys", default-features = false } | ||
| eckit = { path = "../../rust-wrappers-playground/crates/eckit" } | ||
| metkit-sys = { path = "../../rust-wrappers-playground/crates/metkit-sys", default-features = false } | ||
| metkit = { path = "../../rust-wrappers-playground/crates/metkit" } | ||
| eccodes-sys = { path = "../../rust-wrappers-playground/crates/eccodes-sys", default-features = false, features = ["vendored", "eccodes-threads"] } |
Comment on lines
+27
to
+29
| bindman = { path = "../../bindman/bindman" } | ||
| bindman-build = { path = "../../bindman/bindman-build" } | ||
| bindman-utils = { path = "../../bindman/bindman-utils" } |
Comment on lines
+195
to
198
| /// Retrieve data from FDB using a `MarsRequest`. | ||
| /// | ||
| /// * `request` - The request specifying which data to retrieve | ||
| /// Returns an `eckit::DataHandle` opened for reading. | ||
| /// |
Comment on lines
+18
to
+21
| //! let request = metkit::MarsRequestBuilder::new("list") | ||
| //! .with("class", "od") | ||
| //! .with("expver", "0001"); | ||
| //! .with("expver", "0001") | ||
| //! .build(); |
Comment on lines
1388
to
+1392
| } | ||
|
|
||
| // Create FDB handle with optional subtoc configuration | ||
| let user_config = if args.disable_subtocs { | ||
| Some(UserConfig { |
Comment on lines
85
to
87
| fn test_concurrent_readonly_methods() { | ||
| let tmpdir = tempfile::tempdir().expect("tmpdir"); | ||
| let config = create_test_config(tmpdir.path()); | ||
| let fdb = Arc::new(Fdb::open(Some(&config), None).expect("failed to create handle")); | ||
| let fdb = Arc::new(Fdb::open_default().expect("failed to create handle")); | ||
|
|
Comment on lines
109
to
111
| fn test_concurrent_list_operations() { | ||
| let tmpdir = tempfile::tempdir().expect("tmpdir"); | ||
| let config = create_test_config(tmpdir.path()); | ||
| let fdb = Arc::new(Fdb::open(Some(&config), None).expect("failed to create handle")); | ||
| let fdb = Arc::new(Fdb::open_default().expect("failed to create handle")); | ||
|
|
Comment on lines
139
to
141
| fn test_concurrent_axes() { | ||
| let tmpdir = tempfile::tempdir().expect("tmpdir"); | ||
| let config = create_test_config(tmpdir.path()); | ||
| let fdb = Arc::new(Fdb::open(Some(&config), None).expect("failed to create handle")); | ||
| let fdb = Arc::new(Fdb::open_default().expect("failed to create handle")); | ||
|
|
Comment on lines
163
to
165
| fn test_stress_concurrent_access() { | ||
| let tmpdir = tempfile::tempdir().expect("tmpdir"); | ||
| let config = create_test_config(tmpdir.path()); | ||
| let fdb = Arc::new(Fdb::open(Some(&config), None).expect("failed to create handle")); | ||
| let fdb = Arc::new(Fdb::open_default().expect("failed to create handle")); | ||
| let iterations = 50; |
Comment on lines
+208
to
+210
| fn test_concurrent_errors_no_crash() { | ||
| let fdb = Arc::new(Fdb::open_default().expect("failed to create handle")); | ||
|
|
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
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.
Description
Use eckit high-level crate
Contributor Declaration
By opening this pull request, I affirm the following:
🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/fdb/pull-requests/PR-284