search

drogue-iot / drogue-cloud

Cloud Native IoT
https://drogue.io
Apache License 2.0
114 stars 30 forks source link

First stab at porting to actix-web 4.0 #243

Closed jcrossley3 closed 2 years ago

jcrossley3 commented 2 years ago

The route macros now use absolute paths to reference ::actix_web::* so this breaks our idea to re-export those things from a service-api module.

So I think we either need to a) avoid using the macros or 2) declare a dep on actix-web "4" in the workspace members that need the macros.

I took something of a hybrid approach here, but I ran into a type mismatch error in the authentication-service wrapping the optional prometheus middleware.

ctron commented 2 years ago

I think moving await from the macros isn't a bad idea. In some cases the change should really be minimal. And if there is a huge benefit, then adding actix-web as a dependency seems fine to. So :+1: for the idea of a hybrid approach!

ctron commented 2 years ago

So, that was me :grin: … I moved a fork of the Conditional middleware, which allowed to use an Option as middleware. This PR progressed upstream, but our forked code wasn't updated (so far). I moved this into a dedicated crate now, and aligned this with the upstream PR.

I was also able to upgrade two more dependencies from beta to proper released, which got released in the last few hours. Yay!

This should be ready to be merged when the CI comes back green.

ctron commented 2 years ago

/test

drogue-bot commented 2 years ago

Aye, aye, captain! … Triggered System Test worklow!

drogue-bot commented 2 years ago

Test run finished, but did not create a test report: https://github.com/drogue-iot/drogue-cloud-testing/actions/runs/1915249572

ctron commented 2 years ago

/test

drogue-bot commented 2 years ago

Aye, aye, captain! … Triggered System Test worklow!

drogue-bot commented 2 years ago
Total Passed Failed Ignored Filtered Duration
✅ 147 147 0 0 0 38m 2s

Git: https://github.com/drogue-iot/drogue-cloud @ refs/heads/actix-web-4.0.1

Commit: 930e74474b1496f5753f68f026c8411aec7ee915
Author: Jens Reimann <jreimann@redhat.com>
Date: Tue, 01 Mar 2022 08:38:16 -0100

    build: make actix-web-extras optional, conflicts with the frontend

Job: https://github.com/drogue-iot/drogue-cloud-testing/actions/runs/1915364468

Report: https://drogue-iot.github.io/drogue-cloud-testing/test-report/2022/03/01/test-run-1915364468.html

ctron commented 2 years ago

The test failure was already present on main. Will fix it there. Thanks for the PR!