Skip to content

replace tide with axum in rest plugin and remove async_std dependencies#2176

Merged
OlivierHecart merged 11 commits into
eclipse-zenoh:mainfrom
ZettaScaleLabs:rest-axum
May 13, 2026
Merged

replace tide with axum in rest plugin and remove async_std dependencies#2176
OlivierHecart merged 11 commits into
eclipse-zenoh:mainfrom
ZettaScaleLabs:rest-axum

Conversation

@wyfo

@wyfo wyfo commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

🏷️ Label-Based Checklist

Based on the labels applied to this PR, please complete these additional requirements:

Labels: enhancement, dependencies

✨ Enhancement Requirements

Since this PR enhances existing functionality:

  • Enhancement scope documented - Clear description of what is being improved
  • Minimum necessary code - Implementation is as simple as possible, doesn't overcomplicate the system
  • Backwards compatible - Existing code/APIs still work unchanged
  • No new APIs added - Only improving existing functionality
  • Tests updated - Existing tests pass, new test cases added if needed
  • Performance improvement measured - If applicable, before/after metrics provided
  • Documentation updated - Existing docs updated to reflect improvements
  • User impact documented - How users benefit from this enhancement

Remember: Enhancements should not introduce new APIs or breaking changes.

📦 Dependency Updates

Since this PR updates dependencies:

  • Changelog reviewed - Breaking changes in dependency identified and addressed
  • Security advisories checked - No known vulnerabilities in new version
  • License compatible - New version license is compatible with project
  • Tests pass - All tests pass with new dependency versions
  • Transitive deps reviewed - New transitive dependencies checked
  • Version pinned appropriately - Semver range is intentional and correct
  • Update scope limited - Updating one major dependency at a time when possible

Best practice: Keep dependency updates focused and test thoroughly.

Instructions:

  1. Check off items as you complete them (change - [ ] to - [x])
  2. The PR checklist CI will verify these are completed

This checklist updates automatically when labels change, but preserves your checked boxes.

@wyfo wyfo added enhancement Existing things could work better dependencies Pull requests that update a dependency file labels Oct 2, 2025
@wyfo
wyfo requested a review from evshary October 2, 2025 08:29
@codecov

codecov Bot commented Oct 2, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.13131% with 47 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@d879f6c). Learn more about missing BASE report.
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
plugins/zenoh-plugin-rest/src/lib.rs 90.05% 37 Missing ⚠️
plugins/zenoh-plugin-rest/src/config.rs 58.33% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2176   +/-   ##
=======================================
  Coverage        ?   74.02%           
=======================================
  Files           ?      400           
  Lines           ?    60846           
  Branches        ?        0           
=======================================
  Hits            ?    45039           
  Misses          ?    15807           
  Partials        ?        0           

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

@evshary evshary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall, LGTM except for some questions.
However, since we don't have a comprehensive unit test for the REST plugin, I would suggest asking the PaaS team to give it a try. Perhaps some behavior might change after the migration and refactor.


[dependencies]
axum = "=0.8.4"
axum-core = "=0.5.2"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need to declare axum-core explicitly in 1.75?
Also, CI suggests using something similar to

[package.metadata.cargo-machete]
ignored = ["prost"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

axum 0.8.5 bumped the MSRV

Comment thread plugins/zenoh-plugin-rest/src/lib.rs Outdated
let app = Router::new()
.route(
"/{*key_expr}",
get(subscribe_or_query)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it miss the POST?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://zenoh.io/docs/apis/rest/
There is no POST in the official documentation, and POST to make query or subscribe makes no sense.

@evshary

evshary commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

Regarding the typos CI check, let's deal with it in another PR
#2177

@evshary evshary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the tests. LGTM except for the CI failures.

@sashacmc
sashacmc requested a review from oteffahi October 21, 2025 08:20

@oteffahi oteffahi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked integration tests that use rest API, they should still pass.
LGTM.

wyfo and others added 2 commits May 12, 2026 18:13
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@OlivierHecart

Copy link
Copy Markdown
Contributor

The z_serve_sse example is broken.

Btw argument parsing of z_serve_sse is broken. It would be great if it could be fixed as well. Ideally zenoh_examples::CommonArgs would be reused.

@OlivierHecart
OlivierHecart merged commit c2e3520 into eclipse-zenoh:main May 13, 2026
23 of 28 checks passed
@OlivierHecart
OlivierHecart deleted the rest-axum branch May 13, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement Existing things could work better

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants