Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: use Axum instead of Actix #234

Merged
merged 24 commits into from
Apr 4, 2025
Merged

refactor: use Axum instead of Actix #234

merged 24 commits into from
Apr 4, 2025
+2,228 −2,466

Conversation

GuillaumeDeconinck
Copy link
Contributor

@GuillaumeDeconinck GuillaumeDeconinck commented Feb 27, 2025 •
edited by GuillaumeDecMeetsMore
Loading

Changed

  • Convert HTTP layer from Actix-web to Axum
    • Axum is the most popular HTTP framework by a big margin, and is closely integrated with Tokio (most popular async runtime)
    • More future-proof
    • Telemetry setup (logs and APM) has been tested
    • To be honest, Actix was working fine, but I've adapted this during my free time as practice 😄
  • Changed local dev tool cargo-watch to watchexec, as cargo-watch is archived

GuillaumeDeconinck and others added 23 commits February 27, 2025 22:34
@GuillaumeDeconinck
refactor: start using axum

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
c8fc013
@GuillaumeDeconinck
Merge branch 'master' into refactor/use-axum
bacc5a2
@GuillaumeDeconinck
Merge branch 'master' into refactor/use-axum
568bf32
@GuillaumeDeconinck
fix: most issues
a6b4ebe
@GuillaumeDeconinck
refactor: continue adapting stuff for Axum
b5446ab
@GuillaumeDeconinck
feat: change 400 to 422 & fix shutdown
fa30fad
@GuillaumeDeconinck
test: fix setup
8247582
@GuillaumeDeconinck
Merge branch 'master' into refactor/use-axum
027083a
@GuillaumeDecMeetsMore
Merge branch 'master' into refactor/use-axum
99b3027
@GuillaumeDecMeetsMore
fix: format
4e59b4a
@GuillaumeDecMeetsMore
Merge branch 'master' into refactor/use-axum
f3f1103
@GuillaumeDecMeetsMore
feat: add basic tracing/logs
6ccbe2e
@GuillaumeDecMeetsMore
fix: try to improve more tracing
a2c2c6e
@GuillaumeDecMeetsMore
fix: trying to debug tracing
2d004f4
@GuillaumeDecMeetsMore
fix: use info span
fcdd558
@GuillaumeDecMeetsMore
fix: healthcheck shouldn't be traced
9959de4
@GuillaumeDecMeetsMore
fix: try to fix tracing
17d18c1
@GuillaumeDeconinck
feat: update tracing so that propagator is shared
5078ca4
@GuillaumeDecMeetsMore
fix: telemetry http client
c6bd5b1
@GuillaumeDecMeetsMore
Merge branch 'master' into refactor/use-axum
bca9555
@GuillaumeDeconinck
Merge branch 'master' into refactor/use-axum
6b36825
@GuillaumeDeconinck
Merge branch 'master' into refactor/use-axum
6278743
@GuillaumeDeconinck
fix(sdk): avoid instantiating client for each request
ba08c12
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore changed the title refactor: start using axum refactor: use Axum instead of Actix Apr 1, 2025
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore marked this pull request as ready for review April 2, 2025 00:45
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore self-assigned this Apr 2, 2025
mm-zacharydavison
mm-zacharydavison previously approved these changes Apr 4, 2025
View reviewed changes
Copy link

@mm-zacharydavison mm-zacharydavison left a comment

Choose a reason for hiding this comment

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

Kind of hard for me to review meaningfully, but approved nonetheless.
I had one comment.

crates/api/src/account/get_account.rs Outdated

// Refetch the account, as the protect_admin_route uses a cached method
// Meaning that the account could have been deleted in the meantime (or updated)
let account = ctx
Copy link

Choose a reason for hiding this comment

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

Intended to remove?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, not intended 😓

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore enabled auto-merge (squash) April 4, 2025 10:07
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore merged commit 9220183 into meetsmore:master Apr 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore

@mm-zacharydavison mm-zacharydavison

@mm-derek mm-derek

Assignees

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@GuillaumeDeconinck @mm-zacharydavison @GuillaumeDecMeetsMore