Skip to content

Rust bindings - #460

Merged
marcosbento merged 37 commits into
developfrom
rust-bindings
Oct 8, 2026
Merged

marcosbento merged 37 commits into
developfrom
rust-bindings

Conversation

@Choochmeque

@Choochmeque Choochmeque commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Description

Rust bindings for c++ client

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/ecflow/pull-requests/PR-460

@codecov-commenter

codecov-commenter commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.69%. Comparing base (6a6fb14) to head (291114d).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #460      +/-   ##
===========================================
- Coverage    56.71%   56.69%   -0.02%     
===========================================
  Files         1265     1265              
  Lines       105999   105999              
  Branches     15403    15403              
===========================================
- Hits         60114    60095      -19     
- Misses       45885    45904      +19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Choochmeque
Choochmeque marked this pull request as ready for review September 28, 2026 15:22
@Choochmeque
Choochmeque force-pushed the rust-bindings branch 2 times, most recently from c7e614d to 2504d9c Compare September 28, 2026 15:45
@marcosbento

marcosbento commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Hi @Choochmeque

Auditing the Rust bindings (sha 2504d9c), based on existing Python bindings, was difficult! :-)
It doesn't help that I had to learn Rust at the same time, but I think I got a good understanding of the code.

The very large majority of the code is straightforward, but there are a few areas that required more attention.
Here are some comments and observations:

  1. The Client in the Python bindings acts like a "context manager" for the API, meaning that __enter__ and __exit__
    methods are implemented to handle resource management (e.g. self->ch1_drop();).
    In the case of Rust, does it behave similarly? If not, we may need examine how resource management is handled in
    the Rust bindings and ensure that it is consistent with the Python implementation.

  2. A few functions do not really match the Python bindings types (for example, set_host_port(host: str, port: str)
    in Python vs set_host_port(host: &str, port: u16) in Rust). I understand that this is due to the differences in
    type systems, and we should ensure that the Rust bindings are intuitive and easy to use for Rust developers.
    I just wonder if anything could prevent us from later adding any missing "overloads" or alternative function
    signatures to make the Rust API more ergonomic?

  3. A few functions adopted a different naming convention in Rust compared to Python (e.g., get_host() vs host()).
    Is this because of Rust's idiomatic naming conventions or to follow Rust's common practice?
    If so, I would leave it as is; but if not, please match the naming conventions as closely as possible (for the sake
    of consistency!)

  4. enable_ssl() in Python considers the presence of ECF_SSL in the environment, calling enable_ssl_if_defined() or
    enable_ssl(), depending on the presence/value. In Rust, enable_ssl() does not check for the environment variable
    and always calls enable_ssl().
    Please ensure that the Rust bindings behave consistently with the Python implementation, or if there is a particular
    reason for a change, document the difference clearly.

  5. Regarding checkpt(), which does Rust have also configure_checkpt()? Is this the idiomatic way to handle optional
    parameters in Rust? If so, please ensure that the documentation clearly explains how to use these functions and any
    differences from the Python implementation.

  6. Noticed the load_defs_file/text, replace_file/text and the defs_text are slightly different in Rust compared to
    Python. I understand that this is due: 1) because the model objects are out of scope, and 2) because the Rust bindings
    are designed to be more idiomatic for Rust developers. Is this correct?

  7. The Rust binding doesn't cover group(cmd: str). I assume this was intentionally left out. If so, I would agree
    with the decision, as it seems to be a less commonly used function.
    However, I would like to understand if there is a specific reason for its omission; it should be documented.

@marcosbento marcosbento left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at my comment.
Some of the points might require changes.

@Choochmeque

Choochmeque commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author
  1. Rust has no with. The equivalent is Drop, which runs when the value goes out of scope, and Client did not drop its handle there. Now it does: a client that holds a registered handle calls ch1_drop() when dropped, as __exit__ does. An error at that point is ignored, since Drop cannot return one. ch_drop(handle) remains for dropping explicitly. (88afeba)

  2. Nothing prevents it. Rust has no overloading, so an alternative signature is a new method with its own name, and adding one does not break existing callers. For the port, Rust takes u16 and converts it to the string ClientInvoker stores, as Python's set_host_port(host, int) overload does. The (str, str) and "host:port" forms can be added when needed. port() returns a string, like get_port(). Where Python overloads on str / list of paths, the Rust method takes any iterator of strings.

  3. It is the Rust convention: getters carry no get_ prefix (Rust API Guidelines, C-GETTER), so get_host() / get_port() are host() / port(). get_certificate() had kept the prefix and is now certificate() (46c2666). Methods that send a request (get_log, get_file, get_server_defs) keep the Python name. The other differences are debug → set_debug, force_state_recursive → the recursive flag of force_state, and version as a free function.

  4. Fixed in 195427f. The constructors already followed ECF_SSL as Python's do, but enable_ssl() always called the plain enable_ssl(). It now calls enable_ssl_if_defined() when ECF_SSL is set and enable_ssl() otherwise, the same as ClientInvoker_enable_ssl. The doc comment is corrected.

  5. Yes. Rust has no default arguments, so Python's checkpt(mode=..., check_pt_interval=0, check_pt_save_alarm_time=0) is two methods: checkpt() writes the check point now, and configure_checkpt(mode, interval, save_time_alarm) changes the settings, with None leaving a setting unchanged. They end in the same checkPtDefs call, and each doc comment points to the other.

  6. Correct on both. With Defs objects out of scope, definitions cross the API as text or as a file path, and since Rust cannot overload, each Python overload has its own name: load → load_defs_file / load_defs_text, replace → replace_file / replace_text, get_defs → defs_text.

  7. group(commands) is now there (7a18b3e), bound directly to ClientInvoker::group. I had left it out because it takes a command-line string, like the invoke(args) we removed. Its doc comment states that commands which ask for confirmation on the command line (delete, halt, shutdown, terminate) read standard input unless given yes, and that show and why write to standard output.

@marcosbento marcosbento left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to merge!

@marcosbento
marcosbento merged commit 308e123 into develop Oct 8, 2026
151 checks passed
@marcosbento
marcosbento deleted the rust-bindings branch October 8, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants